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

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 angular#54864.
Resolves angular#55028.
Resolves angular#55036.
  • Loading branch information
AndrewKushnir committed Mar 28, 2024
1 parent d15dca0 commit 35931d8
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 @@ -500,6 +501,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 @@ -532,16 +542,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

1 comment on commit 35931d8

@dreamstar-enterprises
Copy link

Choose a reason for hiding this comment

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

Does this mean the issue is close to being fixed?

Please sign in to comment.