Skip to content

Commit

Permalink
fix(core): establish proper injector resolution order for @defer bl…
Browse files Browse the repository at this point in the history
…ocks (#55079)

This commit updates the `@defer` logic to establish proper injector resolution order. More specifically:

- Makes node injectors to be inspected first, similar to how it happens when `@defer` block is not used.
- Adds extra handling for the Router's `OutletInjector`, until we replace it with an `EnvironmentInjector`.

Resolves #54864.
Resolves #55028.
Resolves #55036.

PR Close #55079
  • Loading branch information
AndrewKushnir authored and dylhunn committed Mar 28, 2024
1 parent fb5a288 commit 708ba81
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 8 deletions.
31 changes: 24 additions & 7 deletions packages/core/src/defer/instructions.ts
Expand Up @@ -17,6 +17,7 @@ import {populateDehydratedViewsInLContainer} from '../linker/view_container_ref'
import {PendingTasks} from '../pending_tasks';
import {assertLContainer, assertTNodeForLView} from '../render3/assert';
import {bindingUpdated} from '../render3/bindings';
import {ChainedInjector} from '../render3/component_ref';
import {getComponentDef, getDirectiveDef, getPipeDef} from '../render3/definition';
import {getTemplateLocationDetails} from '../render3/instructions/element_validation';
import {markViewDirty} from '../render3/instructions/mark_view_dirty';
Expand Down Expand Up @@ -502,6 +503,15 @@ export function renderDeferBlockState(
}
}

/**
* Detects whether an injector is an instance of a `ChainedInjector`,
* created based on the `OutletInjector`.
*/
function isRouterOutletInjector(currentInjector: Injector): boolean {
return (currentInjector instanceof ChainedInjector) &&
((currentInjector.injector as any).__ngOutletInjector);
}

/**
* Applies changes to the DOM to reflect a given state.
*/
Expand Down Expand Up @@ -534,16 +544,23 @@ function applyDeferBlockState(
const providers = tDetails.providers;
if (providers && providers.length > 0) {
const parentInjector = hostLView[INJECTOR] as Injector;
const parentEnvInjector = parentInjector.get(EnvironmentInjector);
injector =
parentEnvInjector.get(CachedInjectorService)
.getOrCreateInjector(
tDetails, parentEnvInjector, providers, ngDevMode ? 'DeferBlock 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);
injector = parentEnvInjector.get(CachedInjectorService)
.getOrCreateInjector(
tDetails, parentEnvInjector as EnvironmentInjector, providers,
ngDevMode ? 'DeferBlock Injector' : '');
}
}
const dehydratedView = findMatchingDehydratedView(lContainer, activeBlockTNode.tView!.ssrId);
const embeddedLView = createAndRenderEmbeddedLView(
hostLView, activeBlockTNode, null, {dehydratedView, embeddedViewInjector: injector});
const embeddedLView =
createAndRenderEmbeddedLView(hostLView, activeBlockTNode, null, {dehydratedView, injector});
addLViewToLContainer(
lContainer, embeddedLView, viewIndex, shouldAddViewToDom(activeBlockTNode, dehydratedView));
markViewDirty(embeddedLView);
Expand Down
68 changes: 67 additions & 1 deletion packages/core/test/acceptance/defer_spec.ts
Expand Up @@ -6,10 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ɵPLATFORM_BROWSER_ID as PLATFORM_BROWSER_ID} from '@angular/common';
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, Input, NgModule, NgZone, Pipe, PipeTransform, PLATFORM_ID, QueryList, Type, ViewChildren, ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR} from '@angular/core';
import {getComponentDef} from '@angular/core/src/render3/definition';
import {ComponentFixture, DeferBlockBehavior, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
import {ActivatedRoute, provideRouter, Router, RouterOutlet} from '@angular/router';

/**
* Clears all associated directive defs from a given component class.
Expand Down Expand Up @@ -4151,4 +4152,69 @@ describe('@defer', () => {
.toContain(`<chart>${serviceFromNgModule}|${tokenFromRootComponent}</chart>`);
});
});

describe('Router', () => {
it('should inject correct `ActivatedRoutes` in components within defer blocks', async () => {
@Component({
standalone: true,
imports: [RouterOutlet],
template: '<router-outlet />',
})
class App {
}

@Component({
standalone: true,
selector: 'another-child',
imports: [CommonModule],
template: 'another child: {{route.snapshot.url[0]}}',
})
class AnotherChild {
route = inject(ActivatedRoute);
}

@Component({
standalone: true,
imports: [CommonModule, AnotherChild],
template: `
child: {{route.snapshot.url[0]}}
@defer (on immediate) {
<another-child />
}
`,
})
class Child {
route = inject(ActivatedRoute);
}

const deferDepsInterceptor = {
intercept() {
return () => {
return [dynamicImportOf(AnotherChild, 10)];
};
},
};

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

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

await allPendingDynamicImports();
app.detectChanges();

expect(app.nativeElement.innerHTML).toContain('child: a');
expect(app.nativeElement.innerHTML).toContain('another child: a');
});
});
});
8 changes: 8 additions & 0 deletions packages/router/src/directives/router_outlet.ts
Expand Up @@ -379,6 +379,14 @@ 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.
*/
private __ngOutletInjector = true;

constructor(
private route: ActivatedRoute,
private childContexts: ChildrenOutletContexts,
Expand Down

0 comments on commit 708ba81

Please sign in to comment.