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.

Sign up for GitHub

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

Jump to bottom

feat(animations): Provide a way to lazy load the animations #50738

Closed
wants to merge 3 commits into from
Closed

feat(animations): Provide a way to lazy load the animations #50738

wants to merge 3 commits into from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Jun 15, 2023 •
edited

As part of #50399, this work here is a proposition to allow the lazy loading of animations code that can shave off up to 16KB gzipped of the main bundle !

The PR introduces a new entry point in the platform-browser package : lazy-animations. This has been necessary because the eager providers (BROWSER_ANIMATIONS_PROVIDERS and BROWSER_NOOP_ANIMATIONS_PROVIDERS) were retaining symbols from @angular/animation/browser making the code splitting & lazy loading impossible.

In this entry point, you will find the new provideLazyLoadingAnimations() function which will provide a new RendererFactory2 : AsyncAnimationRendererFactory.

AsyncAnimationRendererFactory will create a new DynamicDelegationRenderer which will delegate to the default renderer by default.
Aside from the renderer, the new factory will lazily import the @angular/animations/browser. Once the package is loaded, the factory will switch the delegate of DynamicDelegationRenderer to an AnimationRenderer.

The side effect of this implementation is that the rendering might not include the styles from the animations defined in components until the module is loaded. This should an acceptable drawback.

Also, I'd like to thank @AndrewKushnir for his help on this PR.

PR Type

  • Feature

Does this PR introduce a breaking change?

  • Nos

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jun 15, 2023
@JeanMeche JeanMeche mentioned this pull request Jun 15, 2023
Closed
@JeanMeche JeanMeche force-pushed the feat/animations-wip branch 2 times, most recently from 1e600bb to 9eb4b94 Compare June 15, 2023 23:08
@angular-robot angular-robot bot removed the detected: feature PR contains a feature commit label Jun 15, 2023
@ngbot ngbot bot added this to the Backlog milestone Jun 16, 2023
@JeanMeche JeanMeche force-pushed the feat/animations-wip branch 3 times, most recently from 22e43cc to a3b6b41 Compare June 17, 2023 23:38
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jun 17, 2023
@JeanMeche JeanMeche force-pushed the feat/animations-wip branch 5 times, most recently from 7456abb to c773b6c Compare June 19, 2023 15:34
@AndrewKushnir AndrewKushnir self-assigned this Jun 19, 2023
@JeanMeche JeanMeche force-pushed the feat/animations-wip branch 3 times, most recently from 7766964 to 52424c2 Compare June 20, 2023 08:32
@angular-robot angular-robot bot removed the detected: feature PR contains a feature commit label Jun 20, 2023
@JeanMeche JeanMeche force-pushed the feat/animations-wip branch 2 times, most recently from a76ee9c to 5b8836b Compare June 20, 2023 15:24
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jun 20, 2023
@JeanMeche JeanMeche force-pushed the feat/animations-wip branch 5 times, most recently from 7acd70a to 62da920 Compare June 22, 2023 16:46
jessicajaniuk
jessicajaniuk approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment •
edited

Choose a reason for hiding this comment

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

The issue was resolved in #51910. Once we have G3 issues resolved, this is good to go.

reviewed-for: fw-animations, fw-core, integration-tests, public-api, size-tracking

devversion
devversion approved these changes Sep 27, 2023
View reviewed changes
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM for Bazel

dylhunn
dylhunn approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from atscott September 27, 2023 16:35
@jessicajaniuk
Copy link
Contributor

TGP

@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Sep 28, 2023
@jessicajaniuk
Copy link
Contributor

TGP is green. This is ready to merge.

@alxhub
Copy link
Member

alxhub commented Sep 29, 2023

This PR was merged into the repository by commit 81e7f5b.

@alxhub alxhub closed this in e753278 Sep 29, 2023
alxhub pushed a commit that referenced this pull request Sep 29, 2023
@JeanMeche @alxhub
refactor(animations): Add integration test for animations/async. (#50738
2c2ccf7 
)

This integration test aims to cover that we do not break the code splitting of the animation module when we use `provideAnimationsAsync()`.

PR Close #50738
alxhub pushed a commit that referenced this pull request Sep 29, 2023
@JeanMeche @alxhub
test(animations): update the integration test (#50738)
81e7f5b 
Let's have the same test app for async and eagerly loaded animations.

PR Close #50738
@robertIsaac
Copy link
Contributor

was it renamed to provideAnimationsAsync?

@jessicajaniuk
Copy link
Contributor

@robertIsaac yes, it was.

@robertIsaac
Copy link
Contributor

I tried it, but saw size increases instead of decrease
The component that have animation is deferred, can it be related?
I can share a reproduce repo if you wish to investigate further

@jessicajaniuk
Copy link
Contributor

@robertIsaac Please file an issue with the repro and we can take a look.

@JeanMeche
Copy link
Member Author

@robertIsaac Please open an new issue with a repro repo and we'll happily investigate that 😊

@JeanMeche JeanMeche deleted the feat/animations-wip branch October 5, 2023 21:58
@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 Nov 12, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
feat(animations): Add the possibility of lazy loading animations code. (
78d43a5 
angular#50738)

`provideLazyLoadedAnimations()` returns providers which allow the lazy loading of the animation module.

Lazy loading of the animation code can shave off up to 16KB gzipped of the main bundle.

PR Close angular#50738
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(animations): Add integration test for animations/async. (ang…
8f30185 
…ular#50738)

This integration test aims to cover that we do not break the code splitting of the animation module when we use `provideAnimationsAsync()`.

PR Close angular#50738
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
test(animations): update the integration test (angular#50738)
eb787b3 
Let's have the same test app for async and eagerly loaded animations.

PR Close angular#50738
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@devversion devversion devversion approved these changes

@dylhunn dylhunn dylhunn approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: animations detected: feature PR contains a feature commit target: major This PR is targeted for the next major release
Projects
None yet
Milestone
v17-candidates
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@JeanMeche @jessicajaniuk @alxhub @robertIsaac @devversion @dylhunn @AndrewKushnir @pkozlowski-opensource