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

Introducing EnvInjector! #45626

Closed
wants to merge 4 commits into from
Closed

Introducing EnvInjector! #45626

wants to merge 4 commits into from

Conversation

alxhub
Copy link
Member

@alxhub alxhub commented Apr 14, 2022

This PR includes a couple commits which introduce the new EnvInjector concept. EnvInjector is a generalized version of NgModuleRef which represents the "module injector" (now renamed to "environment injector"). It's one of the primitives needed to support upcoming standalone components APIs.

@alxhub alxhub force-pushed the standalone-env branch 8 times, most recently from 424d469 to ba03f2f Compare April 14, 2022 22:22
@alxhub alxhub force-pushed the standalone-env branch 3 times, most recently from ca6934c to ab2815d Compare April 15, 2022 00:07
@alxhub alxhub added the target: minor This PR is targeted for the next minor release label Apr 15, 2022
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Looks great 👍 Just left a few minor comments (none of them are blockers, we can update things in followup PRs).

goldens/public-api/core/index.md Outdated Show resolved Hide resolved
goldens/public-api/core/index.md Show resolved Hide resolved
Comment on lines 55 to 58
* created by the framework.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* created by the framework.
*/
* created by the framework.
*
* @publicApi
*/

packages/core/src/di/r3_injector.ts Show resolved Hide resolved
packages/core/src/di/r3_injector.ts Show resolved Hide resolved
packages/core/src/di/r3_injector.ts Show resolved Hide resolved
packages/core/src/di/r3_injector.ts Show resolved Hide resolved
Copy link
Contributor

@atscott atscott 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

goldens/public-api/core/index.md Show resolved Hide resolved
Copy link
Contributor

@atscott atscott 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: size-tracking

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, size-tracking

@pullapprove pullapprove bot requested a review from jelbourn April 15, 2022 20:57
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM 🍪

Just a few non-blocking comments.

goldens/public-api/core/index.md Show resolved Hide resolved
packages/core/src/di/interface/defs.ts Outdated Show resolved Hide resolved
packages/core/src/di/interface/defs.ts Show resolved Hide resolved
packages/core/src/di/scope.ts Outdated Show resolved Hide resolved

// Make sure the INJECTOR token provides this injector.
this.records.set(INJECTOR, makeRecord(undefined, this));

// And `EnvironmentInjector` if the current injector is supposed to be env-scoped.
if (scopes.has('env')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's another instance of env. Is there any way to make these not magic strings?

Copy link
Member Author

Choose a reason for hiding this comment

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

They're sort of by definition magic strings, since they're typically generated by the compiler.

packages/core/src/di/r3_injector.ts Show resolved Hide resolved
packages/core/src/render3/ng_module_ref.ts Outdated Show resolved Hide resolved
packages/core/src/render3/ng_module_ref.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jessicajaniuk jessicajaniuk 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, size-tracking

Previously there was a test ordering issue with the application_module_spec
tests where the value of `getLocaleId()` depended on the order in which
tests ran. Specifically, `setLocaleId()` lower-cases the current locale ID,
so the measured value in a test depended on whether a previous test had
called `setLocaleId()` (the difference between 'en-US' and 'en-us').
@alxhub alxhub force-pushed the standalone-env branch 2 times, most recently from 9d8065d to dcae004 Compare April 15, 2022 22:46
@alxhub alxhub force-pushed the standalone-env branch 2 times, most recently from e173bf1 to c72330f Compare April 15, 2022 23:02
This commit implements the `importProvidersFrom` function that allows
extracting a list of `Provider`s from a list of NgModule types. The
R3Injector which implements DI at the "module" level for Angular is
refactored to use this functionality under the hood.

This commit also implements `INJECTOR_INITIALIZER`, a DI multi-provider
token which is used to run initialization logic when an injector is created.
@@ -138,7 +138,7 @@ export {CssSelectorList, ProjectionSlots} from './interfaces/projection';
export {
setClassMetadata,
} from './metadata';
export {NgModuleFactory, NgModuleRef} from './ng_module_ref';
export {NgModuleFactory, NgModuleRef, createEnvironmentInjector} from './ng_module_ref';

Choose a reason for hiding this comment

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

nit: import without usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an export :)

Choose a reason for hiding this comment

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

OMG, tired brain :-)

This commit exposes a new `EnvironmentInjector` abstraction, which
generalizes the "module injector" concept to injectors that are not based on
NgModules.

An EnvironmentInjector is a conceptual analogue of an `NgModuleRef` - it
represents an injector on the former "module" DI hierarchy in Angular (now
renamed to the "environment injector hierarchy"). Environment injectors are
created via the `createEnvironmentInjector` function from a list of
`Provider`s.

For backwards compatibility with current code using `NgModuleRef`,
`EnvironmentInjector`s are wrapped by an adapter `NgModuleRef`
implementation, so injecting `NgModuleRef` always returns the latest
`EnvironmentInjector`, even if that injector was not based on an NgModule.
Conversely, NgModule-based `NgModuleRef`s created via `createNgModuleRef`
are _also_ `EnvironmentInjector`s.
This commit adds a new internal scope to `R3Injector` for
`EnvironmentInjector`s specifically. This will allow us to scope services to
the environment side of the injector hierarchy specifically, as opposed to
the `'any'` scope which also includes view-side injectors created via
`Injector.create`. For now, this functionality is not exposed publicly, but
is available to use within `@angular/core` only.
Copy link
Contributor

@atscott atscott 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

@alxhub alxhub added the action: merge The PR is ready for merge by the caretaker label Apr 16, 2022
@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Apr 16, 2022
@ngbot ngbot bot added this to the Backlog milestone Apr 16, 2022
@alxhub alxhub added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Apr 16, 2022
@alxhub
Copy link
Member Author

alxhub commented Apr 16, 2022

Caretaker: this has all required approvals

@AndrewKushnir AndrewKushnir requested review from amysorto and removed request for jelbourn and amysorto April 16, 2022 01:37
@dylhunn
Copy link
Contributor

dylhunn commented Apr 18, 2022

This PR was merged into the repository by commit 3578e94.

@dylhunn dylhunn closed this in a5a7fbc Apr 18, 2022
dylhunn pushed a commit that referenced this pull request Apr 18, 2022
This commit implements the `importProvidersFrom` function that allows
extracting a list of `Provider`s from a list of NgModule types. The
R3Injector which implements DI at the "module" level for Angular is
refactored to use this functionality under the hood.

This commit also implements `INJECTOR_INITIALIZER`, a DI multi-provider
token which is used to run initialization logic when an injector is created.

PR Close #45626
dylhunn pushed a commit that referenced this pull request Apr 18, 2022
…#45626)

This commit exposes a new `EnvironmentInjector` abstraction, which
generalizes the "module injector" concept to injectors that are not based on
NgModules.

An EnvironmentInjector is a conceptual analogue of an `NgModuleRef` - it
represents an injector on the former "module" DI hierarchy in Angular (now
renamed to the "environment injector hierarchy"). Environment injectors are
created via the `createEnvironmentInjector` function from a list of
`Provider`s.

For backwards compatibility with current code using `NgModuleRef`,
`EnvironmentInjector`s are wrapped by an adapter `NgModuleRef`
implementation, so injecting `NgModuleRef` always returns the latest
`EnvironmentInjector`, even if that injector was not based on an NgModule.
Conversely, NgModule-based `NgModuleRef`s created via `createNgModuleRef`
are _also_ `EnvironmentInjector`s.

PR Close #45626
dylhunn pushed a commit that referenced this pull request Apr 18, 2022
This commit adds a new internal scope to `R3Injector` for
`EnvironmentInjector`s specifically. This will allow us to scope services to
the environment side of the injector hierarchy specifically, as opposed to
the `'any'` scope which also includes view-side injectors created via
`Injector.create`. For now, this functionality is not exposed publicly, but
is available to use within `@angular/core` only.

PR Close #45626
@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 19, 2022
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: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants