feat(core): support bootstrapping Angular components underneath shadow roots#66782
Open
dgp1130 wants to merge 11 commits intoangular:mainfrom
Open
feat(core): support bootstrapping Angular components underneath shadow roots#66782dgp1130 wants to merge 11 commits intoangular:mainfrom
dgp1130 wants to merge 11 commits intoangular:mainfrom
Conversation
6ffae5d to
855f44b
Compare
d1e53b0 to
c421a8e
Compare
b5ed4ee to
b1189be
Compare
ROOT_STYLE_HOST to configure Angular to bootstrap under a shadow root272a8b9 to
33fe9f5
Compare
Contributor
Author
|
One challenge I'm discovering from looking through g3 failures is that CDK Portal apparently relies on being able to manually move rendered DOM from one location to another. This PR breaks that use case because Angular calls |
efc094b to
eee5a01
Compare
b8859e8 to
ddf0cdc
Compare
…` with a dedicated `InjectionToken` This allows code in `@angular/core` to inject and use `SharedStylesHost`, even though the implementation is defined in `@angular/platform-browser`.
This is necessary for an `RNode` to discover whether it is within the context of a shadow root, which is needed to know where a component's style should be placed. The method is Baseline Widely Available, however we need to treat it as optional for SSR / JSDom contexts where shadow DOM is not supported.
…esHost` When rendering components underneath shadow roots, it is possible to render multiple times, meaning `SharedStylesHost.prototype.addHost` will potentially be called multiple times on the same shadow root. This is reasonable, and we need to make sure that as long as any one component is still using a given shadow root as its host, we should continue applying styles there. Only when all usages of the host are removed do we stop adding styles to it.
This is useful for testing that `addHost` / `removeHost` were called the correct number of times to ensure hosts are not leaking.
3f5e198 to
5439829
Compare
Prior to this change, `ViewRef.prototype.destroy` did not short-circuit if it had already been called. This led to a serious issue in certain dynamic component setups, particularly with `@angular/cdk/portal`. When the CDK attaches a `ComponentPortal`, it explicitly registers a disposal function that calls `ComponentRef.prototype.destroy()`. If the portal outlet itself is destroyed as part of a classic Angular teardown sequence, the native framework teardown will destroy the portal`s parent `ViewContainerRef`, subsequently destroying its child views. However, moments later, the CDK`s `ngOnDestroy` hook executes and forcibly repeats the process by triggering `ComponentRef.prototype.destroy()`. Without idempotency, this second destroy invocation cascaded down into the `LView` cleanup processes (`destroyLView`) again, which led to duplicate style unregistration via `SharedStylesHost.removeHost`. If a host had already been unregistered, `SharedStylesHost` threw errors or suffered internal tracking corruption. Adding `if (this.destroyed) return;` acts as a necessary guarantee that views, and by extension style hosts, are only cleaned up precisely once, preserving cross-layer stability.
…w roots This updates component creation and view attach/detach to identify any containing shadow root and apply required styles to it. Managing this during attach / detach onto a view container seems like the ideal timing for what we need, since a node might theoretically be moved from one shadow root to another post-render. Awkwardly, `ViewEncapsulation.ShadowDom` / `ViewEncapsulation.IsolatedShadowDom` manage tracking of hosts already directly in `dom_renderer.ts`, and components should put their own styles in their own shadow root, not a containing shadow root. So we need to leave this alone for now and only modify the other `ViewEncapsulation` modes. A future refactor should consider unifying all `ViewEncapsulation` modes into a single code path. This is quite inefficient as it adds all styles to all shadow roots, which is excessive and unnecessary. However precisely adding only the required styles is a more complicated refactor. I initially tried to call `SharedStylesHost` via dependency injection, but found that injecting during component destruction does not work reliably, as the injector may already be destroyed. Instead, `SharedStylesHost` is tracked in the `LViewEnvironment` which works well as a single object reference shared amongst all components anyways.
…ter destruction Since `SharedStylesHost` can receive `addHost` and `removeHost` calls after it is destroyed in testing environments (e.g., when fakeAsync triggers CD after teardown), we need to safely no-op these operations to avoid errors.
… `LViewEnvironment` Prior to this change, when style encapsulation fell back from Shadow DOM, it defaulted to the global `document.head` via `getDocument()`. This was problematic for mocked or isolated environments (such as unit tests) where a custom `DOCUMENT` token is provided, but `setDocument` is not called. Because `SharedStylesHost` relies on the node it is passed, using the global `document.head` in a mocked environment could lead to cross-document DOM insertion errors or just failing to style the component correctly. This commit introduces `fallbackDocument` to `LViewEnvironment`. It is captured at bootstrap in `createRootLViewEnvironment` directly from the `DOCUMENT` injection token, since we can't reliably inject during component destruction, falling back to `getDocument()`. This ensures that the correct document context is plumbed through the view layer, guaranteeing stable style injection inside the expected document environment without cross-document pollution.
This tests bootstrapping Angular underneath a shadow root and that styles are applied and removed at the correct locations.
…d on the injector While developing this feature, I attempted to inject a service during component destruction (to call `SharedStylesHost.prototype.removeHost`), however this broke some Material use cases which seem to destroy the injector before the component. Whether this constraint was intended or not, I believe we currently live in a world where component destruction cannot depend on its injection, so I'm enforcing this in a test to be clear about that constraint. This is not intended to imply that this is a _good_ constraint or that we _want_ this to be the case, but it does represent the reality we currently exist in.
…styles when re-adding a host in SharedStylesHost `SharedStylesHost` has a known bug where removing hosts does not remove styles from the DOM and fixing that would be a breaking change. However, this leads to a separate bug where removing a style down to zero usages and then adding an extra one, could duplicate styles on the page. This ensures styles are only appended if they aren't already present on the parent node from a previously leaked style.
5439829 to
8e0fd54
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Local tests seem to be passing, but I'm currently working through some google3 failures.
This PR allows Angular components to be bootstrapped inside of a shadow root and properly apply their styles to that shadow root instead of the document
<head>. This PR is an alternative to #66048, which has been stalled due to complicated tests which we've been struggled to debug, and attempts to solve the same problem by making minimal changes to the frameworks and avoid pulling on more complexity than absolutely necessary.The early commits are just refactoring some internals and setting up the right utilities and types required. The
testcommit with shadow DOM is the ultimate goal of this PR.The main
featcommit is the real work. It tracks style hosts (document<head>or shadow root) during component render and attach/detach to a container. This allows a single component render in a random location to find an ancestor shadow root and track it for styles to be added as appropriate.Based on some discussions with @AndrewKushnir, we believe the right time to do this is during component render xor attach to view container. Since a component could theoretically be moved from one container to another or rendered in a detached state (via
createComponent) and then attached to a specific container later.We use
Node.prototype.getRootNodeto determine whether or not a particular component is within aShadowRoot. We could use some framework-internal data structures, but ultimately we need to support shadow roots outside the root component anyways and unknown to Angular, so we cannot expect to rely on framework data structures to solve this entire problem.