Skip to content

fix(@schematics/angular): migrate secondary entry point style options in use-library-builder - #34468

Merged
clydin merged 2 commits into
angular:mainfrom
thekhegay:use-library-builder-sass
Oct 21, 2026
Merged

clydin merged 2 commits into
angular:mainfrom
thekhegay:use-library-builder-sass

Conversation

@thekhegay

Copy link
Copy Markdown
Contributor

PR Checklist

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

use-library-builder drops style options of secondary entry points
ng-packagr's own scss-paths sample shows it: sub-module/ng-package.json has own styleIncludePaths, after migration sub-module/assets is just gone from includePaths

findSecondaryEntryPoints reads only lib.entryFile from secondary config and then file is deleted, so styleIncludePaths and sass set per entry point are lost (primary ones are migrated)
Small workspace with sub/ng-package.json setting "styleIncludePaths": ["../styles", "./styles"] builds fine with ng-packagr, after migration ng build fails:

✘ [ERROR] Error: Can't find stylesheet to import.
  ╷
2 │ @use 'tokens';
  │ ^^^^^^^^^^^^^
  ╵
  ../../../../projects/lib/sub/b.component.scss 2:1  root stylesheet [plugin angular-sass]

Secondary sass.silenceDeprecations is lost too, so @import deprecation warning comes back

What is the new behavior?

Library builder has one global stylePreprocessorOptions, so include paths of secondary entry points are resolved from their own diretory and merged into includePaths after primary ones, duplicates removed
sass deprecation lists of all entry points are merged same way
Same workspace builds after migration (both packages built from this branch) and styles in output match ng-packagr build

New case in migration_spec.ts sets styleIncludePaths and sass on primary and on testing entry point, it fails on main

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Because options become global, partial with same name in two include paths now resolves to first one for every entry point
If you prefer warning for secondary options instead of merging, I can switch to that

Also lib.cssUrl: "none" is dropped without any message, library builder always inlines relative url() as data URI and has no such option
Should migration warn about it? Can add it here or in seperate PR

… in use-library-builder

migration read only `lib.entryFile` from secondary `ng-package.json` and then deleted it, so its `styleIncludePaths` and `sass` options were lost and next build failed with `Can't find stylesheet to import`
library builder has single global `stylePreprocessorOptions`, so secondary optoins are now merged into it next to primary ones

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the use-library-builder migration schematic to collect and merge styleIncludePaths and sass options from secondary entry points. Feedback on the changes highlights an issue where existing sass options in the workspace configuration (angular.json) are overwritten rather than merged, and a code suggestion is provided to preserve and merge these existing options.

Comment thread packages/schematics/angular/migrations/use-library-builder/migration.ts Outdated
@alan-agius4 alan-agius4 added the target: minor This PR is targeted for the next minor release label Oct 21, 2026
Comment thread packages/schematics/angular/migrations/use-library-builder/migration.ts Outdated
Comment thread packages/schematics/angular/migrations/use-library-builder/migration.ts Outdated
…options in use-library-builder

Co-authored-by: Alan Agius <alanagius@google.com>
@thekhegay
thekhegay requested a review from alan-agius4 October 21, 2026 13:49
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Oct 21, 2026
@clydin
clydin merged commit c32a690 into angular:main Oct 21, 2026
40 checks passed
@clydin

clydin commented Oct 21, 2026

Copy link
Copy Markdown
Member

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @schematics/angular target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants