Skip to content

Migrate packages/compiler and packages/elements - #61566

Closed
devversion wants to merge 11 commits into
angular:mainfrom
devversion:rjs-6
Closed

devversion wants to merge 11 commits into
angular:mainfrom
devversion:rjs-6

Conversation

@devversion

@devversion devversion commented May 21, 2025 •

Copy link
Copy Markdown
Member

See individual commits

Also replaces #61290

@angular-robot angular-robot Bot added the area: build & ci Related the build and CI infrastructure of the project label May 21, 2025
@ngbot ngbot Bot added this to the Backlog milestone May 21, 2025
devversion and others added 7 commits May 27, 2025 12:07
Migrates `packages/compiler` to `ts_project`.
Compiler now would have `.js` files. Those aren't picked up as ESM,
unless we install the `package.json` with `type: module`. Sounds great
on paper, but doesn't work in reality because the way the compiler
packages are available to `api-gen/` is via the old `rules_nodejs`
linker, so the `packages/package.json` wouldn't work; nor do the
`package.json`s of the e.g. compiler-cli package work- because those
already contain the `exports` of the built npm package.

We fix this in a much more reasonable way, and the whole module
resolution problem by leveraging the pnpm linking here. This works as
expected.
Migrates `packages/elements` to `ng_project`.
See associated pull request for more information.
This is necessary to support later versions of e.g. `rules_ts`.
…er-cli

Currently when linking the `@angular/compiler-cli` package, the peer
dependency to `compiler` is not resolved and we are trying to make the
compiler dependency available via `data`. This is unidiomatic and
brittle. This commit fixes this.
The `app_bundle` rule does not work after the migration of
`packages/compiler` to `ts_project` because the `.mjs` extensions are
now missing in the non npm-package output.

This causes runtime errors as `.js` is not recognized as ESM. Switching
to the real npm package for usage, fixes this issue.
@devversion
devversion force-pushed the rjs-6 branch 4 times, most recently from 84fb97b to 7782a6e Compare May 27, 2025 18:58
Since we are going to replace our `app_bundle` rule (custom ESBuild +
Terser pipeline) with the real Angular CLI where shared/lazy/common
chunks may exist, we need to update the symbol extractor to support
multiple files.

We could have just merged all symbols, but this commit tries to do
better by detecting what symbols are loaded eagerly vs. lazily. This
will be very useful for e.g. defer tests or other lazy features we are
introducing in the feature.
Instead of dev-infra maintaining a custom ESBuild + Terser pipeline that
tries to emulate the Angular CLI, we are switching the bundling core
tests to a new rule that really leverages the Angular CLI.

This involves some file renames and small adjustments. In addition, we
leverage the updated symbol tracking rule to output new goldens that can
work with multiple bundle files (as generated by the Angular CLI;
especially with defer and its "lazy" chunks).
…on rule

We are dropping the custom ESBuild and Terser pipeline from dev-infra
and instead leverage the Angular CLI directly. This commit adjusts
the benchmarks to use this new rule.
These are also excluded on CI, and shouldn't run locally either as they
are "manual debug" targets.
@devversion
devversion requested a review from crisbeto May 28, 2025 08:40
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: rc This PR is targeted for the next release-candidate labels May 28, 2025
"identity",
"importProvidersFrom",
"inNotificationPhase",
"includeViewProviders",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting that there are way more symbols here compared to before. Maybe things aren't being tree shaken correctly?

@devversion devversion May 28, 2025 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

good callout. I did check with Charles and it seems to be expected because the Angular CLI purely relies on ESBuild for inlining, while Terser (used previously in CLI and in our custom pipeline) was much more aggressive here.

@devversion
devversion marked this pull request as ready for review May 28, 2025 11:57
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 28, 2025
@devversion

Copy link
Copy Markdown
Member Author

Caretaker note: Please checkout cl/764208851 and then run g3sync. Afterwards, patch cl/764208851 into your sync CL.

@devversion devversion added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: minor This PR is targeted for the next minor release and removed target: rc This PR is targeted for the next release-candidate labels May 28, 2025
@kirjs

kirjs commented May 29, 2025

Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 909e543.

The changes were merged into the following branches: main

@kirjs kirjs closed this in ef44f67 May 29, 2025
kirjs pushed a commit that referenced this pull request May 29, 2025
Compiler now would have `.js` files. Those aren't picked up as ESM,
unless we install the `package.json` with `type: module`. Sounds great
on paper, but doesn't work in reality because the way the compiler
packages are available to `api-gen/` is via the old `rules_nodejs`
linker, so the `packages/package.json` wouldn't work; nor do the
`package.json`s of the e.g. compiler-cli package work- because those
already contain the `exports` of the built npm package.

We fix this in a much more reasonable way, and the whole module
resolution problem by leveraging the pnpm linking here. This works as
expected.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
Migrates `packages/elements` to `ng_project`.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
See associated pull request for more information.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
This is necessary to support later versions of e.g. `rules_ts`.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
…er-cli (#61566)

Currently when linking the `@angular/compiler-cli` package, the peer
dependency to `compiler` is not resolved and we are trying to make the
compiler dependency available via `data`. This is unidiomatic and
brittle. This commit fixes this.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
)

The `app_bundle` rule does not work after the migration of
`packages/compiler` to `ts_project` because the `.mjs` extensions are
now missing in the non npm-package output.

This causes runtime errors as `.js` is not recognized as ESM. Switching
to the real npm package for usage, fixes this issue.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
Since we are going to replace our `app_bundle` rule (custom ESBuild +
Terser pipeline) with the real Angular CLI where shared/lazy/common
chunks may exist, we need to update the symbol extractor to support
multiple files.

We could have just merged all symbols, but this commit tries to do
better by detecting what symbols are loaded eagerly vs. lazily. This
will be very useful for e.g. defer tests or other lazy features we are
introducing in the feature.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
Instead of dev-infra maintaining a custom ESBuild + Terser pipeline that
tries to emulate the Angular CLI, we are switching the bundling core
tests to a new rule that really leverages the Angular CLI.

This involves some file renames and small adjustments. In addition, we
leverage the updated symbol tracking rule to output new goldens that can
work with multiple bundle files (as generated by the Angular CLI;
especially with defer and its "lazy" chunks).

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
…on rule (#61566)

We are dropping the custom ESBuild and Terser pipeline from dev-infra
and instead leverage the Angular CLI directly. This commit adjusts
the benchmarks to use this new rule.

PR Close #61566
kirjs pushed a commit that referenced this pull request May 29, 2025
These are also excluded on CI, and shouldn't run locally either as they
are "manual debug" targets.

PR Close #61566
@angular-automatic-lock-bot

Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Jun 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note PullApprove: disable target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants