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

perf(forms): avoid direct references to the Validators class #41189

Conversation

AndrewKushnir
Copy link
Contributor

Currently the Validators class contains a number of static methods that represent different validators as well as some helper methods. Since class methods are not tree-shakable, any reference to the Validator class retains all of its methods (even if you've used just one).

This commit refactors the code to extract the logic into standalone functions and use these functions in the code instead of referencing them via Validators class. That should make the code more tree-shakable. The Validators class still retains its structure and calls these standalone methods internally to keep this change backwards-compatible.

PR Type

What kind of change does this PR introduce?

  • Other... Please describe: perf-related change.

Does this PR introduce a breaking change?

  • Yes
  • No

@AndrewKushnir
Copy link
Contributor Author

Note: this PR is related to (and partially addresses) #41011.

@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Mar 12, 2021
@AndrewKushnir AndrewKushnir marked this pull request as ready for review March 12, 2021 05:08
@AndrewKushnir
Copy link
Contributor Author

Presubmit.

Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

LGTM!

@AndrewKushnir AndrewKushnir 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 Mar 15, 2021
Currently the `Validators` class contains a number of static methods that represent different validators as well as some helper methods. Since class methods are not tree-shakable, any reference to the `Validator` class retains all of its methods (even if you've used just one).

This commit refactors the code to extract the logic into standalone functions and use these functions in the code instead of referencing them via `Validators` class. That should make the code more tree-shakable. The `Validators` class still retains its structure and calls these standalone methods internally to keep this change backwards-compatible.
@jessicajaniuk jessicajaniuk force-pushed the form-tree-shake-validators-class branch from 04d293e to 966d897 Compare March 15, 2021 17:36
@jessicajaniuk jessicajaniuk added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Mar 15, 2021
@jessicajaniuk
Copy link
Contributor

This has been merged into master only due to conflicts with 11.2.x. @AndrewKushnir can you create a patch-only PR for 11.2.x?

AndrewKushnir added a commit to AndrewKushnir/angular that referenced this pull request Mar 15, 2021
…ar#41189)

Currently the `Validators` class contains a number of static methods that represent different validators as well as some helper methods. Since class methods are not tree-shakable, any reference to the `Validator` class retains all of its methods (even if you've used just one).

This commit refactors the code to extract the logic into standalone functions and use these functions in the code instead of referencing them via `Validators` class. That should make the code more tree-shakable. The `Validators` class still retains its structure and calls these standalone methods internally to keep this change backwards-compatible.

PR Close angular#41189
@ockendenjo
Copy link

I came across this whilst looking through the CHANGELOG and updating Angular

Is this just an internal change or do users of @angular/forms need to change how we use validators?

I have quite a lot of code that looks something like this:

public emailControl = new FormControl("", [Validators.email, Validators.required]);

Does it need to be written in a different way to be tree-shakeable?

@JoostK
Copy link
Member

JoostK commented Apr 8, 2021

@ockendenjo It's an internal only change for now, so your code is not affected (nor can it benefit from this change at the moment). Please see #41257 for details.

@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 May 9, 2021
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: forms area: performance cla: yes cross-cutting: tree-shaking target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants