Skip to content

fix(web-components): harden component lifecycle for SSR/DSD hydration scenarios#35838

Open
radium-v wants to merge 45 commits intomicrosoft:masterfrom
radium-v:users/radium-v/option-checks
Open

fix(web-components): harden component lifecycle for SSR/DSD hydration scenarios#35838
radium-v wants to merge 45 commits intomicrosoft:masterfrom
radium-v:users/radium-v/option-checks

Conversation

@radium-v
Copy link
Copy Markdown
Contributor

@radium-v radium-v commented Mar 4, 2026

Previous Behavior

  • Many component change handlers assumed CSR lifecycle timing — they did not account for scenarios where DOM content already exists (SSR/DSD) and property assignments or slot observations fire during construction or connection, before internal references are available.
  • elementInternals properties were set in change handlers without connection guards, which fails when callbacks fire before the element is connected to the DOM.
  • Button, Slider, and Dialog did not robustly manage disabled state and tabindex transitions.
  • The test harness fast-fixture had diverged from downstream improvements to fixture options and stability handling.
  • Several test files used test.step loops, making individual test case failures harder to isolate in CI.

New Behavior

  • Hardened change handlers and observable callbacks across Avatar, Button, Dropdown, Listbox, Menu, ProgressBar, RadioGroup, RatingDisplay, Tab, Tablist, TextInput, Textarea, Tree, and TreeItem to 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 accessing elementInternals.
  • Improved disabled state and tabindex management in Button, Slider, and AnchorButton.
  • Enhanced fast-fixture test harness with fixture options and stability handling ported from downstream.
  • Refactored tests for Badge, ProgressBar, Text, and Tab from test.step loops into individual parameterized test() calls.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

📊 Bundle size report

✅ No changes found

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

Pull request demo site: URL

@@ -0,0 +1,7 @@
{
Copy link
Copy Markdown

@github-actions github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕵🏾‍♀️ visual changes to review in the Visual Change Report

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 3 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-web-components/MenuList. - Dark Mode.normal.chromium.png 498 Changed
vr-tests-web-components/MenuList. - RTL.2nd selected.chromium.png 17 Changed
vr-tests-web-components/MenuList. - RTL.1st selected.chromium_2.png 39384 Changed
vr-tests-web-components/RadioGroup 1 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-web-components/RadioGroup. - Dark Mode.normal.chromium_1.png 89 Changed

There were 1 duplicate changes discarded. Check the build logs for more information.

@radium-v radium-v force-pushed the users/radium-v/option-checks branch 2 times, most recently from a3d379c to c2bdc26 Compare April 3, 2026 03:42
radium-v added 23 commits April 2, 2026 20:44
…ts for improved readability and maintainability
@radium-v radium-v changed the title fix(web-components): quirks when utilizing DSD fix(web-components): harden component lifecycle for SSR/DSD hydration scenarios Apr 3, 2026
@radium-v radium-v force-pushed the users/radium-v/option-checks branch from c2bdc26 to 6f5d9e3 Compare April 3, 2026 06:07
@radium-v radium-v marked this pull request as ready for review April 3, 2026 06:08
@radium-v radium-v requested a review from a team as a code owner April 3, 2026 06:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 from test.step loops into parameterized individual test() 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

  • childTreeItems is no longer initialized (changed from [] to definite assignment) but many handlers access this.childTreeItems.length without guarding. If an event fires before the slot/ref plumbing assigns childTreeItems, this will throw. Initialize childTreeItems to an empty array to keep these handlers safe.

*/
@attr({ mode: 'boolean' })
disabled = false;
disabled!: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue at play here is this triggering attributeChangedCallback() on the platform.

Copy link
Copy Markdown
Contributor

@davatron5000 davatron5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.enqueue
  • requestAnimationFrame
  • this.$fastController.isConnected
  • waitForConnectedDescendants
  • requestIdleCallback
  • handleChange(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.

Comment on lines +331 to +345
this.$fastController.subscribe(
{
handleChange: (source: any, propertyName: string) => {
if (propertyName === 'isConnected') {
waitForConnectedDescendants(this, () => {
requestAnimationFrame(() => {
this.setTabs();
});
});
}
},
},
'isConnected',
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@marchbox marchbox Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment on lines +322 to +331
const subscriber: Subscriber = {
handleChange: () => {
if (this.$fastController.isConnected) {
this.setValidity();
this.$fastController.unsubscribe(subscriber, 'isConnected');
}
},
};

this.$fastController.subscribe(subscriber, 'isConnected');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@radium-v radium-v self-assigned this Apr 3, 2026
@radium-v
Copy link
Copy Markdown
Contributor Author

radium-v commented Apr 3, 2026

@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 connectedCallback(), which is expected and preferred as per the spec. One core set of issues under the hood with fast-element's use of @observable (and by extension @attr) are that they're using TypeScript's experimental decorators, which get transpiled into function calls which mutate the class prototype. All of this happens before anything even gets set up and registered in FAST.

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 constructor(), and then they trigger the observable -Changed() functions during construction. This is why we've had so many checks for this.$fastController.isConnected in the past - but checking this flag is a point-in-time check, and there's no await or throttling. So even if you set a default value on a property, anything in that -Changed() call has to account for the possibility that the element may not be connected yet, and it may not have even finished contsructing yet.

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 connectedCallback happens when the entire opening tag is present in the DOM, which is usually too early for components which have to be aware of the slotted child elements which by nature can only exist after this opening tag. So any actions performed by the parent on the children have to be throttled or timed so that they can happen after the children are available, and in most cases, after they have also been connected. The irony is that this.$fastController.isConnected becomes true thanks to the super.connectedCallback() call, but the platform's own isConnected happens after the connectedCallback() is finished firing. So there's no actual way, from within the connectedCallback, to do things after connection. Sometimes that means we need to wait a full frame, sometimes we need a microtask, sometimes requestIdleCallback works, sometimes we need to use FAST's own observation system to definitively wait until we're really truly actually 100% connected (lol even then maybe it's not finished? That's where the grab bag of throttling options truly comes into play).

The third issue is that in DSD scenarios, the constructor and connectedCallback happen almost simultaneously, so using one over the other doesn't often matter for default values. But again, you can't do DOM things during the constructor, so you also can't do much during the connectedCallback which is also already happening.

Additionally in DSD, things like slotchange events fire immediately (?), like waaaaaayyyyyyy before any JS is even present on the page to listen for it, if they even fire at all (if a tree falls and nobody can hear it...). So in places where we were relying on that, we now have to augment it to use FAST's built-in directives for ref() and slotted(), which when observed, fire a -Changed() when their values are present. This always happens after the template has been resolved and the DOM can be poked again, so it's safer than some other alternatives. But it's ultimately a clumsy approach that can look like a no-op without proper documentation around why we're waiting for a div in the template to exist as a reference so we can so some stuff that we're already doing in other places.

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 Observable directly. Ideally, this would be something that @observable would just do, but we can't change it now now in fast-element because it would break existing v2 functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants