Skip to content

Commit efc094b

Browse files
dgp1130dgp1130
committed
WIP: fix g3 tests
1 parent 33fe9f5 commit efc094b

File tree

9 files changed

+52
-12
lines changed

9 files changed

+52
-12
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,15 @@ function createRootLViewEnvironment(rootLViewInjector: Injector): LViewEnvironme
182182
}
183183

184184
const sharedStylesHost = rootLViewInjector.get(SHARED_STYLES_HOST, null);
185+
const fallbackHost = rootLViewInjector.get(DOCUMENT, getDocument() as any)?.head ?? null;
185186

186187
return {
187188
rendererFactory,
188189
sanitizer,
189190
changeDetectionScheduler,
190191
ngReflect,
191192
sharedStylesHost,
193+
fallbackHost,
192194
};
193195
}
194196

@@ -327,11 +329,12 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
327329
const rootNode = hostElement.getRootNode?.();
328330
const isShadowRoot =
329331
rootNode && typeof ShadowRoot !== 'undefined' && rootNode instanceof ShadowRoot;
330-
const styleHost = isShadowRoot ? rootNode : getDocument().head;
332+
const fallbackHost = rootLView[ENVIRONMENT].fallbackHost ?? getDocument().head;
333+
const styleHost = isShadowRoot ? rootNode : fallbackHost;
331334

332-
sharedStylesHost.addHost(styleHost);
335+
sharedStylesHost.addHost(styleHost as Node);
333336
(rootLView[ON_DESTROY_HOOKS] ??= []).push(
334-
() => void sharedStylesHost.removeHost(styleHost),
337+
() => void sharedStylesHost.removeHost(styleHost as Node),
335338
);
336339
}
337340
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ export interface LViewEnvironment {
402402
* HTML stylesheets may choose not to provide a `SharedStylesHost`.
403403
*/
404404
sharedStylesHost: SharedStylesHost | null;
405+
406+
/**
407+
* The fallback host node (usually the injected DOCUMENT's head) to attach
408+
* styles to when a specific host (like a ShadowRoot) is not available or
409+
* applicable.
410+
*/
411+
fallbackHost: Node | null;
405412
}
406413

407414
/** 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
@@ -157,6 +157,7 @@ describe('di', () => {
157157
changeDetectionScheduler: null,
158158
ngReflect: false,
159159
sharedStylesHost: new MockSharedStylesHost(),
160+
fallbackHost: document,
160161
},
161162
{} as any,
162163
null,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export function enterViewWithOneDiv() {
7272
changeDetectionScheduler: null,
7373
ngReflect: false,
7474
sharedStylesHost: new MockSharedStylesHost(),
75+
fallbackHost: document,
7576
},
7677
renderer,
7778
null,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ export class ViewFixture {
119119
changeDetectionScheduler: null,
120120
ngReflect: false,
121121
sharedStylesHost: new MockSharedStylesHost(),
122+
fallbackHost: document,
122123
},
123124
hostRenderer,
124125
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)