microsoft / microsoft/fluentui

[Bug]: TagPickerControl cancels its aside-width frame in the effect body, so the width token is written only ~25% of the time

Open Beginner friendly
#36,649 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
20.3k
Forks
2.9k
Avg merge
2d 9h
Merged PRs (30d)
46

Description

Component

TagPicker

Package version

@fluentui/react-tag-picker 9.10.3

React version

18.3.1

Environment
node v22.12.0
@fluentui/react-tag-picker 9.10.3
Chrome 147, production build, StrictMode OFF
Current Behavior

components/TagPickerControl/useTagPickerControl.tsx — a ResizeObserver callback schedules the
only write of --fui-TagPickerControl-aside-width inside a requestAnimationFrame. The
matching cancelAnimationFrame is called from the effect body rather than from the effect's
cleanup, so it races the observer's first callback and frequently cancels the frame that carries the
write.

Measured on one TagPicker, production build, no StrictMode, 12 fresh page loads:

frames that wrote the token resting padding-inline-end input width
today 3 of 12 30px or 12px 258.297 or 276.297
after moving the cancel into the cleanup 12 of 12 12px 276.297

That is an 18px width swing between runs of the same page, with no user input and no code
change between loads.

Expected Behavior

The aside-width token is written on every mount. The control's resting geometry is deterministic.

Reproduction

Load a page containing a TagPicker with an aside (the clear/expand affordance) repeatedly in a
production build with StrictMode off, and read
getComputedStyle(control).getPropertyValue('--fui-TagPickerControl-aside-width') plus the
control's resting padding-inline-end on each load.

⚠ Reproduction trap — this is the single most likely way to wrongly conclude the report is
stale.
Under StrictMode in a dev build the defect is invisible: unpatched measures 12/12,
because React's double-invoke detaches and re-attaches the ref, the observer fires again after the
effect has run, and the write lands. Anyone probing a StrictMode dev Storybook will see correct
behaviour every time.

⚠ Second trap — the mechanism is not what it looks like. This is not targetDocument
identity churn. Instrumented in the same production bundle, effectRuns is 1 on 12 of 12 — the
effect never re-runs. The race is React's passive-effect flush against the observer's first
callback, and the cross-tabulation is one-to-one: rafIdRef.current null → no cancel → 30px;
"set" → cancel → 12px. A maintainer handed the identity-churn story would check, find
targetDocument stable, and reasonably reject the report.

Steps to reproduce
  1. Build a page with a TagPicker for production (NODE_ENV=production), without StrictMode.
  2. Load it 12 times, fresh each time.
  3. Record the control's resting padding-inline-end on each load.
  4. Observe two distinct values across the runs.
Discovery context

Found during pixel-level verification of a styling layer built over
@fluentui/react-headless-components-preview. The layer's parity gate diffs each scene against its
@fluentui/react-components twin at threshold 0, and the TagPicker scene would not go green
deterministically — a scene that passes on some runs and fails on others is a race, not noise, so it
was traced rather than absorbed into a tolerance.

Proposed fix

Move the cancelAnimationFrame from the effect body into the effect's return () => { … }
cleanup. Roughly three lines.

Completeness was measured over four arms × 12 loads on a dev bundle before proposing it:

  • no observer leakobserve() calls equal constructor calls in every arm;
  • no double-fire — the aside frame fires exactly once;
  • no StrictMode regression — 12/12 with the fix;
  • strictly better unmount — today nothing cancels the pending frame, which fires against a
    detached ref and is swallowed by optional chaining.

Two regression tests were added to TagPickerControl.test.tsx, both verified to fail without the
fix
. jsdom cannot reproduce the real timing race, but it can pin the ordering deterministically:
observe() is called from a ref callback, i.e. in the commit phase before React flushes the passive
effect, so a stub observer that invokes its callback synchronously puts a frame in flight by the
time the effect runs. The end-to-end browser probes remain the timing-level gate.

Note for whoever verifies: react-tag-picker's suite reports 1 failed / 181 passed both with and
without
this change — a pre-existing @fluentui/react-icons snapshot drift, unrelated.

A working implementation is included in PR #[WINDMOD-PR-NUMBER] (commit f34b0a1125); happy to
split it into a standalone PR against this issue.

Suggested severity

High - No workaround (the write is internal; a consumer cannot schedule it themselves).

Are you willing to submit a PR to fix?

yes

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in components/TagPickerControl/useTagPickerControl.tsx and inspect the ResizeObserver callback, requestAnimationFrame scheduling, and effect lifecycle. Read the two regression tests in TagPickerControl.test.tsx, run the react-tag-picker suite, and verify the production reproduction: the aside-width token and resting geometry should be deterministic on every mount.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
86/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.