feat(core): introduce EnvironmentProviders wrapper type #47669
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alxhub
force-pushed
the
v15/core/env-providers
branch
2 times, most recently
from
99c2702
to
e953d3d
Compare
alxhub
force-pushed
the
v15/core/env-providers
branch
from
e953d3d
to
5722992
Compare
alxhub
added
cla: yes
target: major
alxhub
force-pushed
the
v15/core/env-providers
branch
2 times, most recently
from
fa5888f
to
1f63122
Compare
This commit introduces the `EnvironmentProviders` interface, but does not yet export it as public API.
This commit modifies `R3Injector` and other code in Angular that deals with providers, to handle `EnvironmentProviders` objects as well as normal `Provider` types. There is no user-visible impact to this change, but it prepares the core of the DI system for the introduction of `EnvironmentProviders` as a public feature.
This commit introduces a new type `EnvironmentProviders` which can be used in contexts where Angular accepted `Provider`s destined for `EnvironmentInjector`s. This includes contexts such as `@NgModule.providers` and `Route.providers`. The new type is useful for preventing such providers from accidentally ending up in `@Component.providers`. It can be used as the return type of provider functions (such as `provideRouter`) to enforce this safety. Because `Provider` allows `any[]` nested arrays, the compile-time safety provided by `EnvironmentProviders` is easily circumvented. However, the runtime shape of `EnvironmentProviders` is not compatible with component injectors and will result in a runtime error if it leaks through (NG0207). A new function `makeEnvironmentProviders` is used to construct this new type from an array of providers. The existing `importProvidersFrom` operation previously returned a very similar type `ImportedNgModuleProviders` which had the same goal. This machinery is switched over to use the new `EnvironmentProviders` interface instead (in fact, `ImportedNgModuleProviders` is now just an alias to `EnvironmentProviders`).
This commit switches `provideRouter()` to return the new `EnvironmentProviders` wrapper type, preventing it from being accidentally (or intentionally) included in `@Component.providers`.
alxhub
force-pushed
the
v15/core/env-providers
branch
from
1f63122
to
640e596
Compare
AndrewKushnir
approved these changes
Oct 7, 2022
| ɵbrand: 'EnvironmentProviders'; | ||
| }; | ||
|
|
||
| export interface InternalEnvironmentProviders extends EnvironmentProviders { |
There was a problem hiding this comment.
nit: may be add a quick comment on why we need this type (I think it's covered in the ɵfromNgModule field description, may be just add a version of it as a comment about this function)?
Comment on lines
+28
to
+29
| * Wrap an array of `Provider`s into `EnvironmentProviders`, preventing them from being accidentally | ||
| * referenced in `@Component in a component injector. |
There was a problem hiding this comment.
Suggested change
| * Wrap an array of `Provider`s into `EnvironmentProviders`, preventing them from being accidentally | |
| * referenced in `@Component in a component injector. | |
| * Wrap an array of `Provider`s into `EnvironmentProviders`, preventing them from being accidentally | |
| * referenced in `@Component` or `@Directive` in a component injector. |
| deepForEach(defProviders, provider => { | ||
| ngDevMode && validateProvider(provider, defProviders as SingleProvider[], injectorType); | ||
| providersOut.push(provider); | ||
| deepForEachProvider(defProviders, provider => { |
There was a problem hiding this comment.
Just curious if we can type it here (vs adding as)?
Suggested change
| deepForEachProvider(defProviders, provider => { | |
| deepForEachProvider(defProviders, (provider: SingleProvider) => { |
pullapprove
bot
requested review from
AndrewKushnir,
jessicajaniuk and
pkozlowski-opensource
AndrewKushnir
approved these changes
Oct 7, 2022
jessicajaniuk
approved these changes
Oct 7, 2022
|
This PR was merged into the repository by commit 07017a7. |
jessicajaniuk
pushed a commit
that referenced
this pull request
Oct 7, 2022
This commit modifies `R3Injector` and other code in Angular that deals with providers, to handle `EnvironmentProviders` objects as well as normal `Provider` types. There is no user-visible impact to this change, but it prepares the core of the DI system for the introduction of `EnvironmentProviders` as a public feature. PR Close #47669
jessicajaniuk
pushed a commit
that referenced
this pull request
Oct 7, 2022
This commit introduces a new type `EnvironmentProviders` which can be used in contexts where Angular accepted `Provider`s destined for `EnvironmentInjector`s. This includes contexts such as `@NgModule.providers` and `Route.providers`. The new type is useful for preventing such providers from accidentally ending up in `@Component.providers`. It can be used as the return type of provider functions (such as `provideRouter`) to enforce this safety. Because `Provider` allows `any[]` nested arrays, the compile-time safety provided by `EnvironmentProviders` is easily circumvented. However, the runtime shape of `EnvironmentProviders` is not compatible with component injectors and will result in a runtime error if it leaks through (NG0207). A new function `makeEnvironmentProviders` is used to construct this new type from an array of providers. The existing `importProvidersFrom` operation previously returned a very similar type `ImportedNgModuleProviders` which had the same goal. This machinery is switched over to use the new `EnvironmentProviders` interface instead (in fact, `ImportedNgModuleProviders` is now just an alias to `EnvironmentProviders`). PR Close #47669
jessicajaniuk
pushed a commit
that referenced
this pull request
Oct 7, 2022
) This commit switches `provideRouter()` to return the new `EnvironmentProviders` wrapper type, preventing it from being accidentally (or intentionally) included in `@Component.providers`. PR Close #47669
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(core): introduce EnvironmentProviders wrapper type
This commit introduces a new type
EnvironmentProviderswhich can be usedin contexts where Angular accepted
Providers destined forEnvironmentInjectors. This includes contexts such as@NgModule.providersand
Route.providers.The new type is useful for preventing such providers from accidentally
ending up in
@Component.providers. It can be used as the return type ofprovider functions (such as
provideRouter) to enforce this safety.Because
Providerallowsany[]nested arrays, the compile-time safetyprovided by
EnvironmentProvidersis easily circumvented. However, theruntime shape of
EnvironmentProvidersis not compatible with componentinjectors and will result in a runtime error if it leaks through (NG0207).
A new function
makeEnvironmentProvidersis used to construct this new typefrom an array of providers.
The existing
importProvidersFromoperation previously returned a verysimilar type
ImportedNgModuleProviderswhich had the same goal. Thismachinery is switched over to use the new
EnvironmentProvidersinterfaceinstead (in fact,
ImportedNgModuleProvidersis now just an alias toEnvironmentProviders).