fix(web-components): harden component lifecycle for SSR/DSD hydration scenarios#35838
fix(web-components): harden component lifecycle for SSR/DSD hydration scenarios#35838radium-v wants to merge 39 commits intomicrosoft:masterfrom
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-web-components/Avatar 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Avatar. - Dark Mode.normal.chromium_1.png | 298 | Changed |
vr-tests-web-components/Badge 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Badge. - Dark Mode.normal.chromium.png | 443 | Changed |
vr-tests-web-components/MenuList 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/MenuList. - Dark Mode.normal.chromium.png | 498 | Changed |
vr-tests-web-components/RadioGroup 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/RadioGroup. - Dark Mode.2nd selected.chromium_3.png | 119 | Changed |
vr-tests-web-components/TextInput 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/TextInput. - Dark Mode.normal.chromium_1.png | 288 | Changed |
a3d379c to
c2bdc26
Compare
…equestAnimationFrame
…ts for improved readability and maintainability
…, and max change handlers
…e aria attribute tests
… disabled fieldset
… and improve internal styles handling
…ize, weight, align, and font attributes
…e elementInternals
c2bdc26 to
6f5d9e3
Compare
There was a problem hiding this comment.
Pull request overview
This PR strengthens @fluentui/web-components behavior in SSR/DSD hydration scenarios by making lifecycle-dependent handlers more resilient when DOM/props/slot content are available earlier than typical CSR timing, and by improving the Playwright fixture + tests to be more stable/diagnosable.
Changes:
- Hardened multiple components’ slot/change handlers and connection timing assumptions to better support SSR/DSD hydration.
- Updated focus/disabled/tabindex behaviors (notably for Tablist/Tab, Slider, Button, Tree/TreeItem, TextInput/TextArea, etc.).
- Improved Playwright test harness (
fast-fixture) and refactored several tests fromtest.steploops into parameterized individualtest()cases.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-components/test/playwright/fast-fixture.ts | Enhances fixture template APIs + stability waiting used by Playwright component tests. |
| packages/web-components/src/utils/typings.ts | Adds a reusable type guard factory for matching custom elements by tag suffix. |
| packages/web-components/src/utils/focusable-element.ts | Extends disabled detection to account for ElementInternals-based ARIA disabled state. |
| packages/web-components/src/tree/tree.base.ts | Adjusts Tree slot wiring and connection-time tabindex initialization for hydration robustness. |
| packages/web-components/src/tree-item/tree-item.base.ts | Adjusts TreeItem slot wiring and tabindex timing to better match hydration lifecycle. |
| packages/web-components/src/textarea/textarea.base.ts | Defers initialization/validity setup to avoid pre-connect reference usage during hydration. |
| packages/web-components/src/text/text.spec.ts | Refactors tests from test.step loops into parameterized test() cases. |
| packages/web-components/src/text-input/text-input.template.ts | Updates slotting approach for label content and start/end templates import path. |
| packages/web-components/src/text-input/text-input.spec.ts | Tweaks focusability in tests (tabindex) for stable focus-related assertions. |
| packages/web-components/src/text-input/text-input.base.ts | Hardens slotted label + control validity handling around connection timing. |
| packages/web-components/src/tablist/tablist.ts | Aligns override signature with base tabsChanged(prev,next) lifecycle. |
| packages/web-components/src/tablist/tablist.base.ts | Improves tab lifecycle (listener cleanup, disabled handling, hydration timing) and sets role/orientation in constructor. |
| packages/web-components/src/tab/tab.ts | Adds ElementInternals role, default slot assignment, and initial aria-disabled handling. |
| packages/web-components/src/tab/tab.spec.ts | Updates tests to use new fixture behavior and improves coverage for new defaults/ARIA. |
| packages/web-components/src/slider/slider.ts | Hardens slider initialization and disabled behavior (including fieldset-disabled scenarios). |
| packages/web-components/src/slider/slider.spec.ts | Adds regression coverage for dragging behavior inside a disabled fieldset. |
| packages/web-components/src/rating-display/rating-display.base.ts | Improves slot reference handling and defers CSS custom property updates for hydration timing. |
| packages/web-components/src/radio/radio.spec.ts | Makes tests explicit about when templates are set and updates createElement coverage setup. |
| packages/web-components/src/radio/radio.options.ts | Adds isRadio helper built on new isCustomElement type guard factory. |
| packages/web-components/src/radio-group/radio-group.ts | Moves slot change handling to observable slotted radios and hardens disabled updates. |
| packages/web-components/src/radio-group/radio-group.template.ts | Switches to slotted() directive for default slot observation. |
| packages/web-components/src/radio-group/radio-group.spec.ts | Refactors orientation tests and tweaks focus-related markup for determinism. |
| packages/web-components/src/progress-bar/progress-bar.spec.ts | Refactors tests from test.step loops into parameterized test() cases. |
| packages/web-components/src/progress-bar/progress-bar.base.ts | Defers indicator width updates and guards ElementInternals updates for hydration timing. |
| packages/web-components/src/option/option.ts | Adjusts selection side effects to avoid connection-timing assumptions. |
| packages/web-components/src/menu/menu.ts | Refactors slot-driven setup to run when descendants are connected/hydrated. |
| packages/web-components/src/listbox/listbox.ts | Reworks id/slot handling to avoid relying on early template-bound attributes. |
| packages/web-components/src/listbox/listbox.template.ts | Adds slot ref capture while removing template-bound id attribute. |
| packages/web-components/src/dropdown/dropdown.base.ts | Defers certain side effects and moves control insertion to connectedCallback for hydration safety. |
| packages/web-components/src/dialog/dialog.ts | Defers applying role/aria-modal changes until internal dialog ref is available. |
| packages/web-components/src/button/button.template.ts | Updates template typing to use BaseButton to match architecture changes. |
| packages/web-components/src/button/button.base.ts | Centralizes tabindex logic and hardens disabled/disabledFocusable behavior. |
| packages/web-components/src/badge/badge.spec.ts | Refactors tests from test.step loops into parameterized test() cases. |
| packages/web-components/src/avatar/avatar.template.ts | Refactors avatar rendering to better handle slotted defaults vs monogram/default icon. |
| packages/web-components/src/avatar/avatar.styles.ts | Updates layout/styling to support new template structure and slotted-content states. |
| packages/web-components/src/avatar/avatar.base.ts | Reworks slot/monogram handling to be safer under hydration and reduce DOM mutation hazards. |
| packages/web-components/src/anchor-button/anchor-button.base.ts | Initializes tabindex on connection to respect author-provided tabindex semantics. |
| packages/web-components/docs/web-components.api.md | Updates API surface documentation to reflect newly added/changed members. |
| change/@fluentui-web-components-50437c2d-8cf5-4ddb-87d0-430812690b0b.json | Adds beachball change entry for the web-components SSR/DSD lifecycle hardening fix. |
Comments suppressed due to low confidence (1)
packages/web-components/src/tree/tree.base.ts:70
childTreeItemsis no longer initialized (changed from[]to definite assignment) but many handlers accessthis.childTreeItems.lengthwithout guarding. If an event fires before the slot/ref plumbing assignschildTreeItems, this will throw. InitializechildTreeItemsto an empty array to keep these handlers safe.
| */ | ||
| @attr({ mode: 'boolean' }) | ||
| disabled = false; | ||
| disabled!: boolean; |
There was a problem hiding this comment.
This would make the property undefined initially if the attribute isn’t there, which isn’t consistent with the platform. I think if a property reflects a boolean attribute, its value should always be true or false.
There was a problem hiding this comment.
I think the issue at play here is this triggering attributeChangedCallback() on the platform.
davatron5000
left a comment
There was a problem hiding this comment.
I understand that the async definitions of server rendered elements (DSD/BTR) is what's informing this PR... but I'm getting a bit overwhelmed with all the different ways we're trying to express "Do this work later". This PR contains all of the following patterns:
Updates.enqueuerequestAnimationFramethis.$fastController.isConnectedwaitForConnectedDescendantsrequestIdleCallbackhandleChange(prop) { switch(prop) { case "isConnected" ... }}this.$fastController.subscribe({ handleChange: ... })<attr>Changed()
This makes it confusing for me to know which pattern to reach for and when I should prefer one over the other. And then almost all of these combine and nest in different ways. Seeing requestAnimationFrame() calls spread everywhere makes me think there's a problem at the event/scheduling level we're duct taping over. No other web-component library/framework that I know of needs this level of case-by-case negotiation.
Should we move waitForConnectedDescendants into FASTElement.connectedCallback()? Does that solve the race condition. Should it also await requestAnimationFrame()? Do we need an updated() callback?
Maybe a Mermaid diagram of where our lifecycle or event loop is breaking down would help me understand. Is this something we'll fix and cleanup in FAST v3? I'd love to understand more.
| this.$fastController.subscribe( | ||
| { | ||
| handleChange: (source: any, propertyName: string) => { | ||
| if (propertyName === 'isConnected') { | ||
| waitForConnectedDescendants(this, () => { | ||
| requestAnimationFrame(() => { | ||
| this.setTabs(); | ||
| }); | ||
| }); | ||
| } | ||
| }, | ||
| }, | ||
| 'isConnected', | ||
| ); | ||
| } |
There was a problem hiding this comment.
This seems like a lot of extra waiting. We subscribe to an isConnected event, we wait for that to change, we wait for descendeants, and then we wait for rAF to set tabs.
There was a problem hiding this comment.
Hopefully we won’t need this once focusgroup PR is in since I simplified lots of tablist logic there (focusgroup observes the owner’s subtree, so it doesn’t matter whether the children are ready when tablist is connected)
| const subscriber: Subscriber = { | ||
| handleChange: () => { | ||
| if (this.$fastController.isConnected) { | ||
| this.setValidity(); | ||
| this.$fastController.unsubscribe(subscriber, 'isConnected'); | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| this.$fastController.subscribe(subscriber, 'isConnected'); |
There was a problem hiding this comment.
This subscriber patten seems different than others. And if we're creating a subscriber in every controlChanged call... do the other ones get cleaned up somewhere or is it fine?
|
@davatron5000 you're right about the confusing differences between approaches and I think the issue is that different things work in different places, and I don't completely know why that's the case - but I blame a few specific things: Pretty much every DOM operation can't happen during construction, so we would typically handle things in the Then, in CSR scenarios, the element gets defined on the custom elements registry and any new elements will construct an instance of that class... but those decorators are all set up and ready to go, even during construction. So when we have things like default property values, those also get transpiled and moved to the So that's the first issue - default property values, combined with transpiled decorators which mutate the class prototype, equals bad times. The second issue is that the The third issue is that in DSD scenarios, the Additionally in DSD, things like We're planning to address all of these issues in FAST v3 and vNext, since all of the approaches that you listed are purely workarounds for a much larger observability-driven lifecycle problem. For now, I'll go back over the changes and see if I can normalize the approaches, especially those that use |
Previous Behavior
elementInternalsproperties were set in change handlers without connection guards, which fails when callbacks fire before the element is connected to the DOM.Button,Slider, andDialogdid not robustly manage disabled state and tabindex transitions.fast-fixturehad diverged from downstream improvements to fixture options and stability handling.test.steploops, making individual test case failures harder to isolate in CI.New Behavior
Avatar,Button,Dropdown,Listbox,Menu,ProgressBar,RadioGroup,RatingDisplay,Tab,Tablist,TextInput,Textarea,Tree, andTreeItemto handle SSR/DSD scenarios where DOM content pre-exists and property assignments fire during construction or connection — guarding against uninitialized references and adding connection checks before accessingelementInternals.Button,Slider, andAnchorButton.fast-fixturetest harness with fixture options and stability handling ported from downstream.Badge,ProgressBar,Text, andTabfromtest.steploops into individual parameterizedtest()calls.