Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(@angular-devkit/build-angular): improve rebuild performance with loaders and prebundling #27146

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

clydin
Copy link
Member

@clydin clydin commented Feb 22, 2024

Processing overhead has been reduce during resolution when the fallback path is enabled for prebundling due to either prebundling exclusions or custom loader options being present. The resolution logic now follows the same behavior as the underlying bundler's external package option and avoids the need for an additional bundler resolution roundtrip. While the overhead of the plugin itself is still present, the execution cost is now greatly reduced. Further optimizations to conditionally include the plugin based on code analysis heuristics may be considered in the future.

Blocked on a separate refactoring to allow typescript path mapping entries to be added to the exclude list of the plugin.

…loaders and prebundling

Processing overhead has been reduce during resolution when the fallback path is enabled
for prebundling due to either prebundling exclusions or custom loader options being present.
The resolution logic now follows the same behavior as the underlying bundler's external
package option and avoids the need for an additional bundler resolution roundtrip. While
the overhead of the plugin itself is still present, the execution cost is now greatly reduced.
Further optimizations to conditionally include the plugin based on code analysis heuristics
may be considered in the future.
@clydin clydin added target: patch This PR is targeted for the next patch release state: blocked labels Feb 22, 2024
Comment on lines +37 to 39
// esbuild's external packages option considers any package-like path as external
// which would exclude paths that are absolute or explicitly relative
build.onResolve({ filter: /^[^./]/ }, async (args) => {
Copy link

@sod sod Feb 24, 2024

Choose a reason for hiding this comment

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

Could this evanw/esbuild#3667 help to exclude this callback to run already in esbuild? Like

const loaders = loaderFileExtensions ? `(${Array.from(loaderFileExtensions).join('|')})$` : '';
const libraries = '^(@angular|rxjs|@ngrx)\/)`';
const exclude = loaders ? `(${libraries}|${loaders})` : libraries;

build.onResolve({ filter: /^[^./]/, exclude }, async (args) => {

Copy link
Collaborator

Choose a reason for hiding this comment

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

No particularly for this use-case.

Copy link

Choose a reason for hiding this comment

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

Seems like my esbuild PR is not happening. At least I got no feedback for 3 months. No urgency on my part, as we patch angular cli via yarn patch to apply #27120 ourself.

@clydin clydin added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: blocked 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