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

feat(@angular/ssr): add modulepreload for lazy-loaded routes #28919

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

alan-agius4
Copy link
Collaborator

Enhance performance when using SSR by adding modulepreload links to lazy-loaded routes. This ensures that the required modules are preloaded in the background, improving the user experience and reducing the time to interactive.

Closes #26484

@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Nov 21, 2024
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: @angular/ssr labels Nov 21, 2024
@alan-agius4 alan-agius4 force-pushed the preload-ssr branch 2 times, most recently from 95f2b19 to 9b4c01a Compare November 21, 2024 11:40
@alan-agius4 alan-agius4 force-pushed the preload-ssr branch 5 times, most recently from 913f62e to 1eb3c20 Compare November 22, 2024 10:22
dgp1130

This comment was marked as resolved.

packages/angular/ssr/src/app.ts Show resolved Hide resolved
packages/angular/ssr/src/app.ts Show resolved Hide resolved
packages/angular/ssr/src/routes/ng-routes.ts Outdated Show resolved Hide resolved
@alan-agius4
Copy link
Collaborator Author

I think my biggest concern is that I'm not sure how comfortable I am .toString()-ing a function and parsing out the dynamic import() statement at runtime in a production server.

This only happens during the build, when building the manifest, on a production server the preloads are already extracted.

@alan-agius4 alan-agius4 force-pushed the preload-ssr branch 3 times, most recently from 2db94b8 to 84c0543 Compare November 29, 2024 10:10
@alan-agius4 alan-agius4 requested a review from dgp1130 November 29, 2024 10:11
@alan-agius4 alan-agius4 force-pushed the preload-ssr branch 8 times, most recently from 212ad1a to d78d2ad Compare December 2, 2024 14:42
@alan-agius4 alan-agius4 force-pushed the preload-ssr branch 7 times, most recently from 509feb7 to b15ae1f Compare December 4, 2024 14:40
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

Just responding to some comment threads, I'll take another look at the code on Monday.

Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

No major concerns on my end. I'm on board with an initial implementation like this and we can work through any edge cases or deoptimizations as they come.

I kind of suspect we'll want to expand the transform to apply to any dynamic import inside a loadComponent / loadChildren function, but I'm happy to keep this constrained for now and expand it later.

Copy link

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @alan-agius4 👍

Enhance performance when using SSR by adding `modulepreload` links to lazy-loaded routes. This ensures that the required modules are preloaded in the background, improving the user experience and reducing the time to interactive.

Closes angular#26484
@alan-agius4 alan-agius4 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 Dec 10, 2024
@alan-agius4 alan-agius4 merged commit 8d7a51d into angular:main Dec 10, 2024
30 checks passed
@alan-agius4 alan-agius4 deleted the preload-ssr branch December 10, 2024 11:39
@angular-automatic-lock-bot
Copy link

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 Jan 10, 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: @angular/ssr detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preload all js bundles required for a specific route
5 participants