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

Change FormBuilder scope to "root" #48237

Closed
angelaki opened this issue Nov 25, 2022 · 12 comments
Closed

Change FormBuilder scope to "root" #48237

angelaki opened this issue Nov 25, 2022 · 12 comments
Labels
area: forms feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature good first issue An issue that is suitable for first-time contributors; often a documentation issue. help wanted An issue that is suitable for a community contributor (based on its complexity/scope). P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone

Comments

@angelaki
Copy link
Contributor

angelaki commented Nov 25, 2022

Which @angular/* package(s) are relevant/related to the feature request?

forms

Description

Not just is the providedIn: module syntax deprecated, is there any reason I do not see that it every was provided with the module and not as a singleton?

FormBuilder

Proposed solution

providedIn: 'root'

Alternatives considered

@angelaki
Copy link
Contributor Author

If there even is a reason it was provided with the module, I'm curious about the new solution? Imho deprecating treeshakable module providers (with no alternative way?) was not a good decision.

@sr5434
Copy link
Contributor

sr5434 commented Nov 26, 2022

How would you import the formBuilder?

sr5434 added a commit to sr5434/angular that referenced this issue Nov 26, 2022
@angelaki
Copy link
Contributor Author

angelaki commented Nov 26, 2022

How would you import the formBuilder?

Why not just provide it in root? Its a singleton / stateless anyway. So no need to import it, just use it?

@ngbot ngbot bot added this to the needsTriage milestone Nov 28, 2022
sr5434 added a commit to sr5434/angular that referenced this issue Nov 28, 2022
This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.
sr5434 added a commit to sr5434/angular that referenced this issue Nov 28, 2022
@alxhub alxhub added feature Issue that requests a new feature feature: under consideration Feature request for which voting has completed and the request is now under consideration labels Nov 30, 2022
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 30, 2022
@alxhub alxhub added good first issue An issue that is suitable for first-time contributors; often a documentation issue. P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent help wanted An issue that is suitable for a community contributor (based on its complexity/scope). labels Nov 30, 2022
sr5434 added a commit to sr5434/angular that referenced this issue Dec 3, 2022
Update formbuilder docs

Implement suggestions

refactor(forms): make FormBuilder classes provided in root

This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.

Revert "Change the scope of the formbuilder(issue angular#48237)"

This reverts commit 4cf5c6b.

Revert "refactor(forms): make FormBuilder classes provided in root"

This reverts commit 20528ae.

Make formBuilder availible in root

Restore profile-editor.component.2.ts
sr5434 added a commit to sr5434/angular that referenced this issue Dec 3, 2022
refactor(forms): make FormBuilder classes provided in root

This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.
sr5434 added a commit to sr5434/angular that referenced this issue Dec 4, 2022
refactor(forms): make FormBuilder classes provided in root

This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.

Remove reactiveFormsModule import

refactor(forms): make FormBuilder classes provided in root

refactor(forms): make FormBuilder classes provided in root

This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.
sr5434 added a commit to sr5434/angular that referenced this issue Dec 4, 2022
This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.
sr5434 added a commit to sr5434/angular that referenced this issue Dec 4, 2022
refactor(forms): make FormBuilder classes provided in root

This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.
@angelaki
Copy link
Contributor Author

angelaki commented Dec 7, 2022

So the old module scope was obviously just wrong? Now I got, why you deprecated this possibility.

Never the less - is there still any posibility to provide a tree-shakable service with a module?

@AndrewKushnir
Copy link
Contributor

Never the less - is there still any posibility to provide a tree-shakable service with a module?

You can annotate services with the @Injectable({providedIn: 'root'}) decorator to make them tree-shakable. Do you have a specific use-case where a connection to an NgModule is required?

@angelaki
Copy link
Contributor Author

Never the less - is there still any posibility to provide a tree-shakable service with a module?

You can annotate services with the @Injectable({providedIn: 'root'}) decorator to make them tree-shakable. Do you have a specific use-case where a connection to an NgModule is required?

Yeah, your probably right... I actually have none. But isn't the use-case imaginable that a module provides a service that can be used but there's no need to use it? Never the less, I'm coming from a module-based approach. And they are getting lesser anyway. Being a component / framework developer, I just don't like the idea of removing feature because they could be misused.

By the way: was there a reason the FormBuilder was provided with the module? Or was this acatually just a misusage?

@sr5434
Copy link
Contributor

sr5434 commented Dec 23, 2022

By the way: was there a reason the FormBuilder was provided with the module? Or was this acatually just a misusage?

I think it might have been to keep everything organized (if every service was provided in root, the documentation would probably be a mess).

@AndrewKushnir
Copy link
Contributor

By the way: was there a reason the FormBuilder was provided with the module? Or was this acatually just a misusage?

@angelaki as @sr5434 mentioned above, this was mainly for the code and docs organization.

@angelaki
Copy link
Contributor Author

angelaki commented Jan 12, 2023

But if I'm not mistaken this would create a new FormBuilder instance for every import of the FormsModule. Not that this would consume much memory, but ... it is stateless anyway, so no need for several instances.

@AndrewKushnir
Copy link
Contributor

@angelaki changing the providedIn to "root" would have no effect: an instance of the FormBuilder class would be created once and stored in the root injector. The instance would be reused across an app when injected.

@angelaki
Copy link
Contributor Author

@AndrewKushnir Yeah, sure. Providing it in root was my accepted recommendation. I mean the earlier behavior, providing it with the module.

trekladyone pushed a commit to trekladyone/angular that referenced this issue Feb 1, 2023
…8245)

refactor(forms): make FormBuilder classes provided in root

This commit updates the FormBuilder classes to provide them in root
instead of using a deprecated pattern of providing a service in a specific
module using the `providedIn` syntax.

Closes angular#48237.

PR Close angular#48245
@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 Feb 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: forms feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature good first issue An issue that is suitable for first-time contributors; often a documentation issue. help wanted An issue that is suitable for a community contributor (based on its complexity/scope). P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

5 participants