Skip to content

perf(forms): make FormBuilder and RadioControlRegistry tree-shakable#41126

Closed
AndrewKushnir wants to merge 2 commits intoangular:masterfrom
AndrewKushnir:tree-shakable-form-builder
Closed

perf(forms): make FormBuilder and RadioControlRegistry tree-shakable#41126
AndrewKushnir wants to merge 2 commits intoangular:masterfrom
AndrewKushnir:tree-shakable-form-builder

Conversation

@AndrewKushnir
Copy link
Contributor

@AndrewKushnir AndrewKushnir commented Mar 9, 2021

This PR makes the FormBuilder and RadioControlRegistry classes tree-shakable by adding the providedIn property to corresponding @Injectable decorators and excluding these classes from NgModule definitions.

Note: this improvement is a part of #41011.

PR Type

What kind of change does this PR introduce?

  • Other... Please describe: performance optimization.

Does this PR introduce a breaking change?

  • Yes
  • No

@ngbot ngbot bot modified the milestone: Backlog Mar 9, 2021
@google-cla google-cla bot added the cla: yes label Mar 9, 2021
@AndrewKushnir
Copy link
Contributor Author

Presubmit.

@mary-poppins
Copy link

You can preview 5482ea3 at https://pr41126-5482ea3.ngbuilds.io/.
You can preview e19a9dd at https://pr41126-e19a9dd.ngbuilds.io/.

@AndrewKushnir AndrewKushnir changed the title perf(forms): make FormBuilder class tree-shakable perf(forms): make FormBuilder and RadioControlRegistry tree-shakable Mar 9, 2021
@AndrewKushnir AndrewKushnir changed the title perf(forms): make FormBuilder and RadioControlRegistry tree-shakable perf(forms): make FormBuilder and RadioControlRegistry tree-shakable Mar 9, 2021
@mary-poppins
Copy link

You can preview ef68a26 at https://pr41126-ef68a26.ngbuilds.io/.

@AndrewKushnir AndrewKushnir force-pushed the tree-shakable-form-builder branch from 2b42649 to acb48ff Compare March 10, 2021 02:16
@mary-poppins
Copy link

You can preview acb48ff at https://pr41126-acb48ff.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 419bbe6 at https://pr41126-419bbe6.ngbuilds.io/.

@AndrewKushnir
Copy link
Contributor Author

Note: this change should land after #41146.

This commit makes the `FormBuilder` class tree-shakable by adding the `providedIn` property to its `@Injectable`
decorator. Now if the `FormBuilder` class is not referenced in application's code, it should not be included into
its production bundle.
@AndrewKushnir AndrewKushnir force-pushed the tree-shakable-form-builder branch from 419bbe6 to 69a0c78 Compare March 13, 2021 00:13
@mary-poppins
Copy link

You can preview d35d95a at https://pr41126-d35d95a.ngbuilds.io/.

This commit makes the `RadioControlRegistry` class tree-shakable by adding the `providedIn` property to its
`@Injectable` decorator. Now if the radio buttons are not used in the app (thus no `RadioControlValueAccessor`
directive is initialized), the `RadioControlRegistry` should not be included into application's prod bundle.
@AndrewKushnir AndrewKushnir force-pushed the tree-shakable-form-builder branch from d35d95a to d0d2d59 Compare March 13, 2021 01:28
@mary-poppins
Copy link

You can preview d0d2d59 at https://pr41126-d0d2d59.ngbuilds.io/.

@AndrewKushnir
Copy link
Contributor Author

Presubmit #2 + Global Presubmit.

},
{
"name": "RadioControlRegistry"
"name": "RadioControlRegistryModule"
Copy link
Contributor Author

@AndrewKushnir AndrewKushnir Mar 14, 2021

Choose a reason for hiding this comment

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

Note to reviewers: this change indicates that the RadioControlRegistry was tree-shaken away, but the RadioControlRegistryModule shows up here as it's used as the providedIn value (see below). Note: the FormBuilder class is not showing up here since it's used in that demo app (forms_reactive) and the forms_template_driven golden file never contained it. I've verified that both symbols are not present in a simple app that don't use radio buttons and doesn't reference FormsBuilder directly.

@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Mar 14, 2021
@AndrewKushnir AndrewKushnir marked this pull request as ready for review March 14, 2021 02:39
This was referenced Mar 18, 2021
@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 Apr 16, 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 cla: yes cross-cutting: tree-shaking target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants