Skip to content

Commit

Permalink
fix(core): make ActivatedRoute inject correct instance inside `@def…
Browse files Browse the repository at this point in the history
…er` blocks (#55374)

`RouterOutlet` uses a unique injector logic that returns a value that correspond to the `ActivatedRoute` token dynamically. This logic breaks when a component/directive/pipe that injects the `ActivatedRoute` is located within a `@defer` block, because defer creates an `EnvironmentInjector` instance, which doesn't have that dynamic logic.

We've added some special handling of the `OutletInjector` in one of the previous commits, but it was incomplete and it was not covering cases when different routes use the same component. This commit updates defer logic to re-establish this dynamic behavior for `ActivatedRoute` by creating an instance of the `OutletInjector` when a parent injector was also an instance of `OutletInjector`.

This fix is a short-term solution and longer term we should find a way to achieve the dynamic behavior that Router relies on, but without adding a special case logic into defer.

Resolves #54864.

PR Close #55374
  • Loading branch information
AndrewKushnir authored and alxhub committed Apr 22, 2024
1 parent aeeb38a commit 5cf14da
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 21 deletions.
35 changes: 31 additions & 4 deletions packages/core/src/defer/instructions.ts
Expand Up @@ -509,9 +509,25 @@ export function renderDeferBlockState(
*/
export function isRouterOutletInjector(currentInjector: Injector): boolean {
return (currentInjector instanceof ChainedInjector) &&
((currentInjector.injector as any).__ngOutletInjector);
(typeof (currentInjector.injector as any).__ngOutletInjector === 'function');
}

/**
* Creates an instance of the `OutletInjector` using a private factory
* function available on the `OutletInjector` class.
*
* @param parentOutletInjector Parent OutletInjector, which should be used
* to produce a new instance.
* @param parentInjector An Injector, which should be used as a parent one
* for a newly created `OutletInjector` instance.
*/
function createRouterOutletInjector(
parentOutletInjector: ChainedInjector, parentInjector: Injector) {
const outletInjector = parentOutletInjector.injector as any;
return outletInjector.__ngOutletInjector(parentInjector);
}


/**
* Applies changes to the DOM to reflect a given state.
*/
Expand Down Expand Up @@ -544,18 +560,29 @@ function applyDeferBlockState(
const providers = tDetails.providers;
if (providers && providers.length > 0) {
const parentInjector = hostLView[INJECTOR] as Injector;

// Note: we have a special case for Router's `OutletInjector`,
// since it's not an instance of the `EnvironmentInjector`, so
// we can't inject it. Once the `OutletInjector` is replaced
// with the `EnvironmentInjector` in Router's code, this special
// handling can be removed.
const parentEnvInjector = isRouterOutletInjector(parentInjector) ?
parentInjector :
parentInjector.get(EnvironmentInjector);
const isParentOutletInjector = isRouterOutletInjector(parentInjector);
const parentEnvInjector =
isParentOutletInjector ? parentInjector : parentInjector.get(EnvironmentInjector);

injector = parentEnvInjector.get(CachedInjectorService)
.getOrCreateInjector(
tDetails, parentEnvInjector as EnvironmentInjector, providers,
ngDevMode ? 'DeferBlock Injector' : '');

// Note: this is a continuation of the special case for Router's `OutletInjector`.
// Since the `OutletInjector` handles `ActivatedRoute` and `ChildrenOutletContexts`
// dynamically (i.e. their values are not really stored statically in an injector),
// we need to "wrap" a defer injector into another `OutletInjector`, so we retain
// the dynamic resolution of the mentioned tokens.
if (isParentOutletInjector) {
injector = createRouterOutletInjector(parentInjector as ChainedInjector, injector);
}
}
}
const dehydratedView = findMatchingDehydratedView(lContainer, activeBlockTNode.tView!.ssrId);
Expand Down
42 changes: 30 additions & 12 deletions packages/core/test/acceptance/defer_spec.ts
Expand Up @@ -9,7 +9,6 @@
import {CommonModule, ɵPLATFORM_BROWSER_ID as PLATFORM_BROWSER_ID} from '@angular/common';
import {ApplicationRef, Attribute, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentRef, createComponent, DebugElement, Directive, EnvironmentInjector, ErrorHandler, getDebugNode, inject, Injectable, InjectionToken, Injector, Input, NgModule, NgZone, Pipe, PipeTransform, PLATFORM_ID, QueryList, Type, ViewChildren, ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR} from '@angular/core';
import {isRouterOutletInjector} from '@angular/core/src/defer/instructions';
import {ChainedInjector} from '@angular/core/src/render3/component_ref';
import {getComponentDef} from '@angular/core/src/render3/definition';
import {NodeInjector} from '@angular/core/src/render3/di';
import {getInjectorResolutionPath} from '@angular/core/src/render3/util/injector_discovery_utils';
Expand Down Expand Up @@ -4161,6 +4160,14 @@ describe('@defer', () => {
it('should inject correct `ActivatedRoutes` in components within defer blocks', async () => {
let routeCmpNodeInjector;

const TokenA = new InjectionToken<string>('TokenA');

@NgModule({
providers: [{provide: TokenA, useValue: 'nested'}],
})
class MyModuleA {
}

@Component({
standalone: true,
imports: [RouterOutlet],
Expand All @@ -4172,11 +4179,12 @@ describe('@defer', () => {
@Component({
standalone: true,
selector: 'another-child',
imports: [CommonModule],
template: 'another child: {{route.snapshot.url[0]}}',
imports: [CommonModule, MyModuleA],
template: 'another child: {{route.snapshot.url[0]}} | token: {{tokenA}}',
})
class AnotherChild {
route = inject(ActivatedRoute);
tokenA = inject(TokenA);
constructor() {
routeCmpNodeInjector = inject(Injector);
}
Expand All @@ -4186,14 +4194,16 @@ describe('@defer', () => {
standalone: true,
imports: [CommonModule, AnotherChild],
template: `
child: {{route.snapshot.url[0]}}
child: {{route.snapshot.url[0]}} |
token: {{tokenA}}
@defer (on immediate) {
<another-child />
}
`,
})
class Child {
route = inject(ActivatedRoute);
tokenA = inject(TokenA);
}

const deferDepsInterceptor = {
Expand All @@ -4206,30 +4216,38 @@ describe('@defer', () => {

TestBed.configureTestingModule({
providers: [
{provide: TokenA, useValue: 'root'},
{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID},
{provide: ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR, useValue: deferDepsInterceptor},
provideRouter([
{path: 'a', component: Child},
{path: 'b', component: Child},
]),
],
});
clearDirectiveDefs(Child);

const app = TestBed.createComponent(App);
await TestBed.inject(Router).navigateByUrl('/a?x=1');
await TestBed.inject(Router).navigateByUrl('/a');
app.detectChanges();

await allPendingDynamicImports();
app.detectChanges();

expect(app.nativeElement.innerHTML).toContain('child: a');
expect(app.nativeElement.innerHTML).toContain('another child: a');
expect(app.nativeElement.innerHTML).toContain('child: a | token: root');
expect(app.nativeElement.innerHTML).toContain('another child: a | token: nested');

// Navigate to `/b`
await TestBed.inject(Router).navigateByUrl('/b');
app.detectChanges();

await allPendingDynamicImports();
app.detectChanges();

// Verify that the first non-NodeInjector refers to the chained injector,
// which represents OutletInjector.
const path = getInjectorResolutionPath(routeCmpNodeInjector!);
const firstEnvInjector = path.find(inj => !(inj instanceof NodeInjector))!;
expect(isRouterOutletInjector(firstEnvInjector)).toBe(true);
// Expect that `ActivatedRoute` information get updated inside
// of a component used in a `@defer` block.
expect(app.nativeElement.innerHTML).toContain('child: b | token: root');
expect(app.nativeElement.innerHTML).toContain('another child: b | token: nested');
});
});
});
21 changes: 16 additions & 5 deletions packages/router/src/directives/router_outlet.ts
Expand Up @@ -380,12 +380,23 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {

class OutletInjector implements Injector {
/**
* A special flag that allows to identify the `OutletInjector` without
* referring to the class itself. This is required as a temporary solution,
* to have a special handling for this injector in core. Eventually, this
* injector should just become an `EnvironmentInjector` without special logic.
* This injector has a special handing for the `ActivatedRoute` and
* `ChildrenOutletContexts` tokens: it returns corresponding values for those
* tokens dynamically. This behavior is different from the regular injector logic,
* when we initialize and store a value, which is later returned for all inject
* requests.
*
* In some cases (e.g. when using `@defer`), this dynamic behavior requires special
* handling. This function allows to identify an instance of the `OutletInjector` and
* create an instance of it without referring to the class itself (so this logic can
* be invoked from the `core` package). This helps to retain dynamic behavior for the
* mentioned tokens.
*
* Note: it's a temporary solution and we should explore how to support this case better.
*/
private __ngOutletInjector = true;
private __ngOutletInjector(parentInjector: Injector) {
return new OutletInjector(this.route, this.childContexts, parentInjector);
}

constructor(
private route: ActivatedRoute,
Expand Down

0 comments on commit 5cf14da

Please sign in to comment.