-
Notifications
You must be signed in to change notification settings - Fork 27.2k
Fix bootstrapping inside shadow roots #66048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
ea5d430
6253eac
65cbb53
691199a
46beccb
0ab3817
1087c0d
1a644c8
1ced740
69a1a81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
StyleRoot
This fixes an issue whereby bootstrapping an Angular component within a shadow root would incorrectly apply its styles to outer document `HTMLHeadElement` instead of the containing shadow root.
The main change is that applying styles can no longer be done during renderer creation as it was done previously, because at that time we don't know where the component is being rendered and therefore what `StyleRoot` should contain its styles. We must defer style application until component render / attach, because it is only then that we know the broader context of where the component exists and therefore which `StyleRoot` which hold its styles. Instead, the `applyStyles` call is moved from renderer creation to when the component renders (for basic `<app-foo />` usage in a template) xor view attach (for more complicated `@if (...) { <app-foo /> }` and similar use cases).
This is done primarily by refactoring `dom_renderer.ts` and `shared_styles_host.ts` to apply styles based on a given `StyleRoot` parameter. This allows any style to target a specific location in the DOM and align better with the Shadow DOM model where style location is important.
We actually find the correct element `StyleRoot` via `Node.prototype.getRootNode`, which returns either `document` (if the `Node` is attached to light DOM) or the containing `ShadowRoot` object (if it exists within shadow DOM). This is able to discover shadow roots outside the root node of a render tree and support a many-to-many relationship between component renders and style roots. It also optimally supports `ViewEncapsulation.ExperimentalIsolatedShadowDom` which will only add its styles to shadow roots it is rendered within and will remove those styles when all instances of that component have been detached from the shadow root. `ViewEncapsulation.ShadowDom` is left behaviorally unchanged for backwards compatibility.- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,16 +87,28 @@ function* walkChildren(parent: LView | LContainer): Generator<LView | LContainer | |
| } | ||
| } | ||
|
|
||
| /** Combine multiple iterables into a single stream with the same ordering. */ | ||
| export function* concat<T>(...iterables: Array<Iterable<T>>): Iterable<T> { | ||
| for (const iterable of iterables) { | ||
| yield* iterable; | ||
| } | ||
| } | ||
|
|
||
| /** Returns the {@link StyleRoot} where styles for the component should be applied. */ | ||
| export function getStyleRoot(lView: LView): StyleRoot | undefined { | ||
| // DOM emulation does not support shadow DOM and `Node.prototype.getRootNode`, so we | ||
| // need to feature detect and fallback even though it is already Baseline Widely | ||
| // Available. In theory, we could do this only on SSR, but Jest, Vitest, and other | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a performance concern for this fallback where we're doing an injector lookup on for every lView creation and move? Should we consider patching the domino adapter to avoid this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm hesitant to patch Domino ourselves for this, especially given that we need to support JSDom for Jest too, so we can't really assume we're on Domino. I guess the patch is the same either way, but I'd rather not add a polyfill for this. Can you help me understand the performance implications of an injector lookup in this context? Would that really be meaningfully slower than walking the DOM tree up to the root? There's potentially a path to storing the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I really don't know, to be honest... |
||
| // Node testing solutions lack DOM emulation as well. | ||
|
dgp1130 marked this conversation as resolved.
|
||
| if (!Node.prototype.getRootNode) { | ||
| const injector = lView[INJECTOR]; | ||
| const doc = injector.get(DOCUMENT); | ||
| return doc; | ||
| // TODO: Can't use injector during destroy because it is destroyed before the | ||
| // component. Is it ok to depend on the `document` global? If not, might need to | ||
| // change the contract of `getStyleRoot` and inject `DOCUMENT` prior to | ||
| // destruction. | ||
| // const injector = lView[INJECTOR]; | ||
| // const doc = injector.get(DOCUMENT); | ||
|
|
||
| return document; | ||
| } | ||
|
|
||
| const renderer = lView[RENDERER]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a reminder