angular/components

bug(Material): Relative CDK SASS imports break Yarn PnP compatibility

dgerhardt opened this issue · 6 comments

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

With the packaging changes in Angular 13 and further optimizations in 13.1, projects can now be successfully built using Yarn's Plug'n'Play module strategy. Projects using Angular Material components can also be build as long as they only use a prebuilt CSS theme. Once @angular/material is imported to the project's SCSS to customize a theme (following https://material.angular.io/guide/theming#custom-themes-with-sass), the build breaks.

It looks like the issue is caused by relative paths to files in the cdk directory (e.g. https://github.com/angular/components/blob/13.1.1/src/material/core/_core.scss#L1) which is no longer a sibling in the node_modules directory with PnP enabled.

Related CLI issue: angular/angular-cli#16980

Reproduction

Reproduction repo: https://github.com/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro

Steps for manual reproduction (w/o the repo):

  1. Create a new Angular project.
  2. Import Material to the SCSS (@use '@angular/material' as mat;).
  3. Use Yarn 3.2 as package manager (yarn set version canary while still in RC phase)
  4. Make sure that PnP is enabled in .yarnrc.yml (nodeLinker: pnp)
  5. yarn && yarn build

Expected Behavior

The build should succeed without SASS errors.

Actual Behavior

The build fails with an SASS error:

HookWebpackError: Module build failed (from ./.yarn/__virtual__/sass-loader-virtual-a07e2721d3/0/cache/sass-loader-npm-12.4.0-3d3847fd35-0f7ca3633e.zip/node_modules/sass-loader/dist/cjs.js):
SassError: Can't find stylesheet to import.
  ╷
1 │ @use '../../cdk/overlay';
  │ ^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  .yarn/__virtual__/@angular-material-virtual-f113869c44/0/cache/@angular-material-npm-13.1.1-701561aec7-d592b46c92.zip/node_modules/@angular/material/core/_core.scss 1:1  @forward
  .yarn/__virtual__/@angular-material-virtual-f113869c44/0/cache/@angular-material-npm-13.1.1-701561aec7-d592b46c92.zip/node_modules/@angular/material/_index.scss 18:1     @use
  src/styles.scss 2:1                                                                                                                                                       root stylesheet
    at tryRunOrWebpackError (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/__virtual__/webpack-virtual-47b0d1e1c6/0/cache/webpack-npm-5.65.0-da658c1b49-221ab8ccd4.zip/node_modules/webpack/lib/HookWebpackError.js:88:9)
    at __webpack_require_module__ (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/__virtual__/webpack-virtual-47b0d1e1c6/0/cache/webpack-npm-5.65.0-da658c1b49-221ab8ccd4.zip/node_modules/webpack/lib/Compilation.js:4979:12)
    at __webpack_require__ (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/__virtual__/webpack-virtual-47b0d1e1c6/0/cache/webpack-npm-5.65.0-da658c1b49-221ab8ccd4.zip/node_modules/webpack/lib/Compilation.js:4936:18)
    at /home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/__virtual__/webpack-virtual-47b0d1e1c6/0/cache/webpack-npm-5.65.0-da658c1b49-221ab8ccd4.zip/node_modules/webpack/lib/Compilation.js:5007:20
    at symbolIterator (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/cache/neo-async-npm-2.6.2-75d6902586-deac9f8d00.zip/node_modules/neo-async/async.js:3485:9)
    at done (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/cache/neo-async-npm-2.6.2-75d6902586-deac9f8d00.zip/node_modules/neo-async/async.js:3527:9)
    at Hook.eval [as callAsync] (eval at create (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/cache/tapable-npm-2.2.1-8cf5ff3039-3b7a1b4d86.zip/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:15:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/cache/tapable-npm-2.2.1-8cf5ff3039-3b7a1b4d86.zip/node_modules/tapable/lib/Hook.js:18:14)
    at /home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/__virtual__/webpack-virtual-47b0d1e1c6/0/cache/webpack-npm-5.65.0-da658c1b49-221ab8ccd4.zip/node_modules/webpack/lib/Compilation.js:4914:43
    at symbolIterator (/home/dgerhardt/angular13.1-yarn3.2-pnp-sass-loader-repro/angular-material-yarn-pnp/.yarn/cache/neo-async-npm-2.6.2-75d6902586-deac9f8d00.zip/node_modules/neo-async/async.js:3482:9)

Environment

  • Angular: 13.1.1
  • CDK/Material: 13.1.1
  • Package Manager: yarn 3.2.0-rc.8 (with nodeLinker=pnp)

Interesting! It should be possible switching the imports to module imports, but it's not yet clear how easy that can be landed within Google. Also we'd need a lint rule/test enforcing this (which should be trivial)

Just brought this up in the team meeting. We will be looking into switching to actual module imports, instead of using relative cross-package imports. We also will need a lint rule that enforces this consistently.

Little update: I've started working on this but it's currently blocked on some refactorings I need to land in the Bazel Sass rules (so that we could actually use module imports in our sources as well). It will take a little longer to sort out the API details with the Sass team (but I might work around this blocker in the interim)

Awesome. Thanks for the update.

Here is the final proposal/PR: #24536.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.