feat(core): introduce `runInInjectionContext` and deprecate prior version #49396

alxhub · 2023-03-10T19:14:42Z

With the introduction of EnvironmentInjector, we added an operation to run a function with access to inject tokens from that injector. This operation only worked for EnvironmentInjectors and not for element/node injectors.

This commit deprecates EnvironmentInjector.runInContext in favor of a standalone API runInInjectionContext, which supports any type of injector.

DEPRECATION:

EnvironmentInjector.runInContext is now deprecated, with runInInjectionContext functioning as a direct replacement:

// Previous method version (deprecated):
envInjector.runInContext(fn);
// New standalone function:
runInInjectionContext(envInjector, fn);

JeanMeche · 2023-03-10T21:08:11Z

Wouldn't that solve #47566 ?

alfaproject · 2023-03-12T15:46:34Z

packages/core/src/di/contextual.ts

+ * @returns the return value of the function, if any
+ * @publicApi
+ */
+export function runInInjectionContext<ReturnT>(injector: Injector, fn: () => ReturnT): ReturnT {


OOC, is this ReturnT a Google stylistic suggestion? Never really saw this outside Google in any major project anyway. Traditionally this would be only T if it's just 1 type generic, or more specifically here TReturn, but never the other way around o:

fwiw: 400ish vs 12K occurences on GH

atscott

reviewed-for: public-api, fw-core

jessicajaniuk

reviewed-for: fw-core, public-api

…sion With the introduction of `EnvironmentInjector`, we added an operation to run a function with access to `inject` tokens from that injector. This operation only worked for `EnvironmentInjector`s and not for element/node injectors. This commit deprecates `EnvironmentInjector.runInContext` in favor of a standalone API `runInInjectionContext`, which supports any type of injector. DEPRECATED: `EnvironmentInjector.runInContext` is now deprecated, with `runInInjectionContext` functioning as a direct replacement: ```typescript // Previous method version (deprecated): envInjector.runInContext(fn); // New standalone function: runInInjectionContext(envInjector, fn); ```

AndrewKushnir

👍

AndrewKushnir · 2023-03-13T17:37:53Z

packages/core/src/di/r3_injector.ts

@@ -327,7 +328,7 @@ export class R3Injector extends EnvironmentInjector {
    return `R3Injector[${tokens.join(', ')}]`;
  }

-  private assertNotDestroyed(): void {
+  assertNotDestroyed(): void {


Suggested change

assertNotDestroyed(): void {

/** @internal */

assertNotDestroyed(): void {

R3Injector is already internal / not exported as public.

AndrewKushnir · 2023-03-13T17:45:15Z

goldens/public-api/core/index.md

@@ -528,6 +528,7 @@ export abstract class EnvironmentInjector implements Injector {
    abstract get<T>(token: ProviderToken<T>, notFoundValue?: T, flags?: InjectFlags): T;
    // @deprecated (undocumented)
    abstract get(token: any, notFoundValue?: any): any;
+    // @deprecated
    abstract runInContext<ReturnT>(fn: () => ReturnT): ReturnT;


I think it'd be great to reuse the runInInjectionContext function within the EnvironmentInjector.runInContext implementation (to avoid code duplication):

angular/packages/core/src/di/r3_injector.ts

Lines 217 to 228 in 7dbe328

override runInContext<ReturnT>(fn: () => ReturnT): ReturnT {

this.assertNotDestroyed();

const previousInjector = setCurrentInjector(this);

const previousInjectImplementation = setInjectImplementation(undefined);

try {

return fn();

} finally {

setCurrentInjector(previousInjector);

setInjectImplementation(previousInjectImplementation);

}

}

This isn't possible because runInInjectionContext depends on R3Injector so that would be a circular reference.

alxhub · 2023-03-14T16:26:00Z

This PR was merged into the repository by commit 0814f20.

angular-automatic-lock-bot · 2023-04-14T00:08:03Z

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-robot bot added the detected: feature PR contains a feature commit label Mar 10, 2023

alxhub force-pushed the fw/reactivity/runininjectioncontext branch from cbce19a to 021be49 Compare March 10, 2023 19:23

alfaproject reviewed Mar 12, 2023

View reviewed changes

alxhub force-pushed the fw/reactivity/runininjectioncontext branch from 021be49 to 042b67a Compare March 13, 2023 16:42

angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Mar 13, 2023

alxhub marked this pull request as ready for review March 13, 2023 16:46

pullapprove bot requested review from AndrewKushnir, jessicajaniuk and pkozlowski-opensource March 13, 2023 16:46

alxhub assigned atscott Mar 13, 2023

atscott approved these changes Mar 13, 2023

View reviewed changes

jessicajaniuk approved these changes Mar 13, 2023

View reviewed changes

alxhub force-pushed the fw/reactivity/runininjectioncontext branch from 042b67a to 4548b4b Compare March 13, 2023 17:26

AndrewKushnir approved these changes Mar 13, 2023

View reviewed changes

alxhub added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Mar 13, 2023

alxhub closed this in 0814f20 Mar 14, 2023

angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): introduce `runInInjectionContext` and deprecate prior version #49396

feat(core): introduce `runInInjectionContext` and deprecate prior version #49396

alxhub commented Mar 10, 2023

JeanMeche commented Mar 10, 2023

alfaproject Mar 12, 2023

JeanMeche Mar 12, 2023

atscott left a comment

jessicajaniuk left a comment

AndrewKushnir left a comment

AndrewKushnir Mar 13, 2023

alxhub Mar 13, 2023 •

edited

AndrewKushnir Mar 13, 2023

alxhub Mar 13, 2023

alxhub commented Mar 14, 2023

angular-automatic-lock-bot bot commented Apr 14, 2023

	assertNotDestroyed(): void {
	/** @internal */
	assertNotDestroyed(): void {

	override runInContext<ReturnT>(fn: () => ReturnT): ReturnT {
	this.assertNotDestroyed();

	const previousInjector = setCurrentInjector(this);
	const previousInjectImplementation = setInjectImplementation(undefined);
	try {
	return fn();
	} finally {
	setCurrentInjector(previousInjector);
	setInjectImplementation(previousInjectImplementation);
	}
	}

feat(core): introduce runInInjectionContext and deprecate prior version #49396

feat(core): introduce runInInjectionContext and deprecate prior version #49396

Conversation

alxhub commented Mar 10, 2023

JeanMeche commented Mar 10, 2023

alfaproject Mar 12, 2023

Choose a reason for hiding this comment

JeanMeche Mar 12, 2023

Choose a reason for hiding this comment

atscott left a comment

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

AndrewKushnir Mar 13, 2023

Choose a reason for hiding this comment

alxhub Mar 13, 2023 • edited

Choose a reason for hiding this comment

AndrewKushnir Mar 13, 2023

Choose a reason for hiding this comment

alxhub Mar 13, 2023

Choose a reason for hiding this comment

alxhub commented Mar 14, 2023

angular-automatic-lock-bot bot commented Apr 14, 2023

feat(core): introduce `runInInjectionContext` and deprecate prior version #49396

feat(core): introduce `runInInjectionContext` and deprecate prior version #49396

alxhub Mar 13, 2023 •

edited