Skip to content

Commit eee5a01

Browse files
dgp1130dgp1130
committed
WIP: fix g3 tests
1 parent 5f8bd95 commit eee5a01

9 files changed

Lines changed: 52 additions & 12 deletions

File tree

‎packages/core/src/render3/component_ref.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ function createRootLViewEnvironment(rootLViewInjector: Injector): LViewEnvironme
185185
}
186186

187187
const sharedStylesHost = rootLViewInjector.get(SHARED_STYLES_HOST, null);
188+
const fallbackHost = rootLViewInjector.get(DOCUMENT, getDocument() as any)?.head ?? null;
188189

189190
return {
190191
rendererFactory,
@@ -193,6 +194,7 @@ function createRootLViewEnvironment(rootLViewInjector: Injector): LViewEnvironme
193194
ngReflect,
194195
tracingService,
195196
sharedStylesHost,
197+
fallbackHost,
196198
};
197199
}
198200

@@ -362,11 +364,12 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
362364
const rootNode = hostElement.getRootNode?.();
363365
const isShadowRoot =
364366
rootNode && typeof ShadowRoot !== 'undefined' && rootNode instanceof ShadowRoot;
365-
const styleHost = isShadowRoot ? rootNode : getDocument().head;
367+
const fallbackHost = rootLView[ENVIRONMENT].fallbackHost ?? getDocument().head;
368+
const styleHost = isShadowRoot ? rootNode : fallbackHost;
366369

367-
sharedStylesHost.addHost(styleHost);
370+
sharedStylesHost.addHost(styleHost as Node);
368371
(rootLView[ON_DESTROY_HOOKS] ??= []).push(
369-
() => void sharedStylesHost.removeHost(styleHost),
372+
() => void sharedStylesHost.removeHost(styleHost as Node),
370373
);
371374
}
372375
}

‎packages/core/src/render3/interfaces/view.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,13 @@ export interface LViewEnvironment {
406406
* HTML stylesheets may choose not to provide a `SharedStylesHost`.
407407
*/
408408
sharedStylesHost: SharedStylesHost | null;
409+
410+
/**
411+
* The fallback host node (usually the injected DOCUMENT's head) to attach
412+
* styles to when a specific host (like a ShadowRoot) is not available or
413+
* applicable.
414+
*/
415+
fallbackHost: Node | null;
409416
}
410417

411418
/** Flags associated with an LView (saved in LView[FLAGS]) */

‎packages/core/src/render3/view/container.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ export function addLViewToLContainer(
126126
const rootNode = host.getRootNode?.();
127127
const isShadowRoot =
128128
rootNode && typeof ShadowRoot !== 'undefined' && rootNode instanceof ShadowRoot;
129-
sharedStylesHost.addHost(isShadowRoot ? rootNode : getDocument().head);
129+
const fallbackHost = lView[ENVIRONMENT].fallbackHost ?? getDocument().head;
130+
sharedStylesHost.addHost((isShadowRoot ? rootNode : fallbackHost) as Node);
130131
}
131132

132133
// When in hydration mode, reset the pointer to the first child in
@@ -166,14 +167,20 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView |
166167
const viewToDetach = lContainer[indexInContainer];
167168

168169
if (viewToDetach) {
169-
// Undo the `SharedStylesHost` registration.
170-
const sharedStylesHost = viewToDetach[ENVIRONMENT].sharedStylesHost;
171-
if (sharedStylesHost) {
172-
const host = viewToDetach[HOST] ?? lContainer[NATIVE];
173-
const rootNode = host?.getRootNode?.();
174-
const isShadowRoot =
175-
rootNode && typeof ShadowRoot !== 'undefined' && rootNode instanceof ShadowRoot;
176-
sharedStylesHost.removeHost(isShadowRoot ? rootNode : getDocument().head);
170+
const host = viewToDetach[HOST] ?? lContainer[NATIVE];
171+
172+
// Defensive check, sometimes a view may be detached multiple times through direct
173+
// lifecycle management in user code.
174+
if (host.isConnected) {
175+
const sharedStylesHost = viewToDetach[ENVIRONMENT].sharedStylesHost;
176+
if (sharedStylesHost) {
177+
// Undo the `SharedStylesHost` registration.
178+
const rootNode = host.getRootNode?.();
179+
const isShadowRoot =
180+
rootNode && typeof ShadowRoot !== 'undefined' && rootNode instanceof ShadowRoot;
181+
const fallbackHost = viewToDetach[ENVIRONMENT].fallbackHost ?? getDocument().head;
182+
sharedStylesHost.removeHost((isShadowRoot ? rootNode : fallbackHost) as Node);
183+
}
177184
}
178185

179186
const declarationLContainer = viewToDetach[DECLARATION_LCONTAINER];

‎packages/core/src/render3/view_ref.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
107107
}
108108

109109
destroy(): void {
110+
if (this.destroyed) return;
111+
110112
if (this._appRef) {
111113
this._appRef.detachView(this);
112114
} else if (this._attachedToViewContainer) {

‎packages/core/test/render3/di_spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ describe('di', () => {
158158
ngReflect: false,
159159
tracingService: null,
160160
sharedStylesHost: new MockSharedStylesHost(),
161+
fallbackHost: document,
161162
},
162163
{} as any,
163164
null,

‎packages/core/test/render3/instructions/shared_spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export function enterViewWithOneDiv() {
7373
ngReflect: false,
7474
tracingService: null,
7575
sharedStylesHost: new MockSharedStylesHost(),
76+
fallbackHost: document,
7677
},
7778
renderer,
7879
null,

‎packages/core/test/render3/view_fixture.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ export class ViewFixture {
120120
ngReflect: false,
121121
tracingService: null,
122122
sharedStylesHost: new MockSharedStylesHost(),
123+
fallbackHost: document,
123124
},
124125
hostRenderer,
125126
null,

‎packages/platform-browser/src/browser.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ const BROWSER_MODULE_PROVIDERS: Provider[] = [
268268
{provide: EVENT_MANAGER_PLUGINS, useClass: KeyEventsPlugin, multi: true},
269269
DomRendererFactory2,
270270
{provide: SHARED_STYLES_HOST, useClass: SharedStylesHost},
271+
// TODO: Only remains for backwards compat, should be removed.
272+
{provide: SharedStylesHost, useExisting: SHARED_STYLES_HOST},
271273
EventManager,
272274
{provide: RendererFactory2, useExisting: DomRendererFactory2},
273275
{provide: XhrFactory, useClass: BrowserXhr},

‎packages/platform-browser/src/dom/shared_styles_host.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
121121
*/
122122
private readonly hosts = new Map<Node, number>();
123123

124+
/** Whether this instance has been destroyed. */
125+
private destroyed = false;
126+
124127
constructor(
125128
@Inject(DOCUMENT) private readonly doc: Document,
126129
@Inject(APP_ID) private readonly appId: string,
@@ -202,9 +205,16 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
202205
removeElements(elements);
203206
}
204207
this.hosts.clear();
208+
this.destroyed = true;
205209
}
206210

207211
addHost(hostNode: Node): void {
212+
// Adding a host after destruction will have no effect and is likely a bug in the caller.
213+
// However, some testing scenarios with fake async appear to trigger CD after `TestBed`
214+
// teardown, meaning Angular may render new components (and then immediately destroy them)
215+
// after the application is destroyed, so we have to allow this to happen and no-op.
216+
if (this.destroyed) return;
217+
208218
const existingUsage = this.hosts.get(hostNode) ?? 0;
209219
if (existingUsage === 0) {
210220
// Add existing styles to new host
@@ -220,6 +230,12 @@ export class SharedStylesHost implements ɵSharedStylesHost, OnDestroy {
220230
}
221231

222232
removeHost(hostNode: Node): void {
233+
// In some scenarios (such as an explicit `ApplicationRef.prototype.destroy` call),
234+
// this instance's `ngOnDestroy` method may be called before a component is destroyed and
235+
// attempts to remove its own styles. In this case, we need to no-op to avoid throwing an
236+
// error.
237+
if (this.destroyed) return;
238+
223239
const usage = this.hosts.get(hostNode);
224240
if (typeof ngDevMode !== 'undefined' && ngDevMode && usage === undefined) {
225241
throw new Error('Attempted to remove a host which was not added.');

0 commit comments

Comments
 (0)