Repository navigation
fix(core): don't overwrite static attributes of hydrated elements - #71200
aminesbdev wants to merge 1 commit into
Conversation
`elementLikeStartShared` called `setupStaticAttributes` even when the element was claimed from the server-rendered DOM instead of being created. That function writes the `class` and `style` attributes directly, so on an element with a static `class` or `style` it dropped anything added during SSR, for example a class added with `Renderer2.addClass` by `RouterLinkActive`. The class came back only once the directive ran again on the client, which caused a visible flash. Static attributes are now only set up for elements that were just created, like `appendChild` right below. Hydrated elements keep what the server rendered.
JeanMeche
left a comment
JeanMeche
left a comment
There was a problem hiding this comment.
Thanks for looking into this as a follow-up to #70828!
While skipping setupStaticAttributes on hydrated elements looks appealing on the surface, doing so in elementLikeStartShared introduces several @angular/core regressions, changes the hydration baseline contract in undesirable ways, and still leaves RouterLinkActive (#70828) broken in key scenarios:
- Breaks static host animation triggers (
@trigger) and@.disabledduring hydration:setupStaticAttributes->setUpAttributesdoesn't just write DOM attributes — it dispatches@-prefixed static host entries (isAnimationProp) torenderer.setProperty(...), which is not serialized in SSR HTML and is required on the client to register triggers / disable animations inAnimationRenderer/AsyncAnimationRenderer. ngSkipHydrationhost & root component inconsistencies:wasLastNodeCreated()isfalsefor a<my-comp ngSkipHydration>host element itself (becauseenterSkipHydrationBlockruns afterlastNodeWasCreated), whileComponentFactory.createstill callssetupStaticAttributesunconditionally on hydrated root hosts.- Turns server-only imperative DOM mutations into permanent untracked client state (and drops client static
hostAttrs/attrswhenever SSR/prerendered HTML differs). - Does not actually solve
RouterLinkActive(#70828): CSR and client-created views still flash;rla.isActive(falseduring hydration) becomes out of sync with the.activeDOM (causingNG0500hydration mismatches if templates branch on#rla="routerLinkActive"); and SSG/prerendered shells can keep stale.active/aria-currentpermanently when initial navigation cancels/errors or when[routerLinkActiveOptions]="null".
Left detailed inline comments on both files below.
| if (wasLastNodeCreated()) { | ||
| setupStaticAttributes(lView[RENDERER], native as RElement, tNode); | ||
| } |
There was a problem hiding this comment.
Guarding setupStaticAttributes with wasLastNodeCreated() here introduces three issues in @angular/core:
1. Breaks static host animation triggers (@trigger) and @.disabled on hydrated elements
setupStaticAttributes -> setUpAttributes (packages/core/src/render3/util/attrs_utils.ts) does not only call renderer.setAttribute: when isAnimationProp(attrName) is true (@ prefix), it calls renderer.setProperty(native, attrName, attrVal).
- Static host entries like
host: {'@myTrigger': '', '@.disabled': 'true'}are compiled byparseHostBindingsintohostAttrs->tNode.mergedAttrs(rather than update-pass bindings). - During SSR,
setPropertysets an in-memory property / hitsNoopAnimationEngine— nothing is serialized into the HTML. - On the client,
AnimationRenderer.setProperty(andAsyncAnimationRenderer.setProperty's replay queue) relies on that call to register the trigger viaengine.process(...)or invokedisableAnimations(el, value)(andDefaultDomRenderer2.setPropertyrelies on it to throwNG05105in dev mode when animation providers are missing). SkippingsetupStaticAttributessilently drops static host@triggerregistration and@.disabledon hydrated elements.
2. Also skips setupStaticAttributes on <my-comp ngSkipHydration> host elements
In locateOrCreateElementNodeImpl (packages/core/src/render3/instructions/element.ts):
const isNodeCreationMode = !canHydrateNode(lView, tNode);andlastNodeWasCreated(isNodeCreationMode);run first, whileisInSkipHydrationBlock()is stillfalse— settingwasLastNodeCreated()tofalse.enterSkipHydrationBlock(tNode)andclearElementContents(native)only run afterwards on that element.
So contrary to the PR description, the ngSkipHydration host element itself has wasLastNodeCreated() === false and now skips setupStaticAttributes despite opting out of hydration.
3. Inconsistent with hydrated root component hosts
ComponentFactory.create (packages/core/src/render3/component_ref.ts) calls setupStaticAttributes(hostRenderer, hostElement, hostTNode) unconditionally even when retrieveHydrationInfo enables hydration on the root view, so root component hosts still overwrite SSR attributes/classes/styles while child elements do not.
| if (isPlatformServer(inject(PLATFORM_ID))) { | ||
| const renderer = inject(Renderer2); | ||
| const element = inject(ElementRef).nativeElement; | ||
| renderer.addClass(element, 'active'); | ||
| renderer.setStyle(element, 'color', 'red'); | ||
| } |
There was a problem hiding this comment.
This test highlights the hydration-contract problem with this approach, as well as why preserving raw SSR attributes doesn't actually resolve RouterLinkActive (#70828):
1. Codifies permanent SSR/CSR DOM divergence
Running renderer.addClass / renderer.setStyle exclusively inside if (isPlatformServer(...)) and asserting that active and color: red remain on the client after hydration stabilizes turns a server/client mismatch into permanent, untracked DOM state that Angular has no awareness of. Conversely, skipping setupStaticAttributes means any client-side static hostAttrs / attrs not already present in the server HTML (e.g., rolling-deploy version skew against CDN-cached SSR/SSG HTML, browser-conditional directives, or custom Renderer2 wrappers) are never applied.
2. Why RouterLinkActive (#70828) still breaks under this approach
RouterLinkActive loses its active class during hydration because RouterLinkActive itself waits until NavigationEnd (if (!this.links || !this.router.navigated) return;) to compute its state on the client:
- CSR & client-rendered views (
@if,@defer,ngSkipHydration): still flash inactive during initial navigation. rla.isActiveis out of sync with the DOM (and can triggerNG0500): During hydration,RouterLinkActiveinitializes_isActive = false, while the DOM keeps.activefrom SSR. Any template binding to#rla="routerLinkActive"— such as[class.bold]="rla.isActive"or@if (rla.isActive) { ... }— will either still flash or fail hydration withNG0500(HYDRATION_NODE_MISMATCH) because SSR renderedrla.isActive === truewhile client hydration evaluatesfalse.- Stale
.active/aria-currentstuck forever on SSG / prerendered shells:- If
/is prerendered withHomeactive and the browser loads/adminwhere an initial guard cancels (NavigationCancel) or errors (NavigationError),router.navigatedremainsfalseandNavigationEndnever fires — leavingHomepermanently.activewitharia-current="page". - Even when
NavigationEnddoes succeed on/admin, if[routerLinkActiveOptions]="null"is set,RouterLinkActive.update()hitsif (this.routerLinkActiveOptions === null && !this._isActive) return;and bails out early (since_isActiveinitialized tofalseon the client), never removing the SSR.activeclass oraria-currentattribute.
- If
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
When an element has a static
classorstyleattribute in its template, hydration replaces the server-rendered value with the static one. Anything added during SSR throughRenderer2(addClass,setStyle) is lost until the code that added it runs again on the client.The server renders
class="nav-link active". After hydration the link only hasclass="nav-link", andactivecomes back at the firstNavigationEnd, so the link flashes. Without the staticclassattribute,tNode.classesisnulland the class survives.elementLikeStartSharedskipsappendChildwhenwasLastNodeCreated()isfalse, but callssetupStaticAttributeseither way.writeDirectClassandwriteDirectStylethen callsetAttribute, which assumes a new, empty element.Issue Number: N/A
What is the new behavior?
setupStaticAttributesonly runs when the element was just created. Hydrated elements keep what the server rendered. Elements created on the client (no hydration,ngSkipHydration, client-only views) go through the same path as before.I added a test in
full_app_hydration_spec.ts: a directive adds a class and a style withRenderer2on the server only, on an element with a staticclassandstyle. It fails without the change.Does this PR introduce a breaking change?
Other information
This came up in #70828, which worked around the flash inside
RouterLinkActive. I checked an app with aRouterLinkActivelink and a lazy route that keeps the initial navigation pending: with this change, the server-renderedactiveclass stays on the link through hydration untilNavigationEnd, without any change to the directive.