ui: Stepper warns about duplicate step values at registration time by bogiii · Pull Request #111567 · Automattic/wp-calypso

bogiii · 2026-06-11T13:48:50Z

Part of the follow-up work for #111036 review feedback (#111036 (comment), #111036 (comment), #111036 (comment)).

Proposed Changes

Move the duplicate-step-value dev warning from Stepper.Root into useStepRegistration, where it can actually fire. The previous warning loop in root.tsx was dead code: registration silently de-duped steps, so the steps array it inspected could never contain duplicates.
Track a per-value mount count in registerStep: only the first registration adds a state entry (later ones warn), and the entry is removed only when the last mounted instance for that value deregisters. Transient duplicates in either unmount order no longer drop a still-mounted step or skew indices and totalSteps.
Simplify the updateStep comparison to a plain shallow compare. The union-keys machinery was needed when registration meta carried ReactNode snapshots; meta is now flat scalars, and the key-count check still catches fields removed between renders (e.g. status).
Remove redundant children/className from StepperPanelProps; ComponentProps<'div'> already provides them.
Add tests: three DOM-level regression tests for duplicate handling, and five hook-level characterization tests for updateStep (written against the old comparison and kept green through the refactor).

Why are these changes being made?

Review feedback on feat(ui): add Stepper component to @automattic/ui #111036 pointed out that the duplicate-value check could never throw, that the updateStep comparison could be simplified after the registration-model refactor, and that the panel prop type duplicated ComponentProps<'div'> fields.
This keeps the de-duping (one entry per value protects index and count integrity) while making the dev warning real, and fixes a latent bug where unmounting one duplicate instance deregistered the step entirely.

Testing Instructions

Run yarn test-packages packages/ui/src/stepper and confirm all tests pass, including the "Stepper dev warnings" block and use-step-registration.tsx.
Run yarn typecheck-packages to confirm the StepperPanelProps change compiles.
Optionally, in Storybook, render a stepper with two steps sharing the same value and confirm a one-time console warning "Two steps share value ..." appears in dev mode.

Pre-merge Checklist

github-actions · 2026-06-11T13:49:28Z

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-183750

Jetpack Cloud Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-183750&env=jetpack

Automattic for Agencies Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-183750&env=a8c-for-agencies

Dashboard Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-183750&env=dashboard

matticbot · 2026-06-11T14:16:48Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/stepper-duplicate-step-warning on your sandbox.

The duplicate-value warning in Stepper.Root was dead code: registration de-duped steps before the warning loop could ever see a duplicate. Move the warning into useStepRegistration where it can actually fire, and return a no-op deregister for rejected duplicates so unmounting one no longer removes the original step registration. Follow-up to #111036 review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The Stepper component is still unreleased, so a bug fix to it needs no changelog entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes Stepper duplicate-step-value handling by moving the duplicate-value dev warning to step registration (where duplicates are actually detected) and ensuring rejected duplicate registrations don’t deregister the original step on unmount. It also updates tests to cover the warning and the corrected unmount behavior.

Changes:

Move duplicate-step-value dev warning from Stepper.Root to useStepRegistration.
Return a no-op deregister function for rejected duplicate step registrations to avoid removing the original step.
Add unit tests for the registration-time warning and for preserving the original step when a duplicate unmounts.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/ui/src/stepper/use-step-registration.ts	Adds registration-time duplicate detection and changes deregistration behavior for duplicates.
packages/ui/src/stepper/test/index.tsx	Adds tests for the duplicate-value warning and for duplicate unmount not affecting the original.
packages/ui/src/stepper/root.tsx	Removes now-dead duplicate scanning warning and documents that duplicates are handled at registration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…p a step A rejected duplicate previously got a no-op deregister, so if the originally-accepted instance unmounted while the duplicate stayed mounted, the value lost its registration entirely. Track a per-value mount count and remove the entry only when the last mounted instance deregisters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

The union-keys comparison in updateStep was needed when registration meta carried ReactNode snapshots; meta is now flat scalars, so a plain shallow compare (with a key-count check for removed fields) is enough. Also drop children/className from StepperPanelProps since ComponentProps<'div'> already provides them. Follow-up to #111036 review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Document the stale-metadata limitation when duplicate step values overlap - Rename use-step-registration test to .ts (no JSX) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…etadata Instead of keying steps by value, each Stepper.Step now registers under a unique useId. The public steps list is derived as one entry per value from the first still-mounted instance, so a duplicate value can no longer override or leave stale metadata on the surviving step. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Order derived steps by each value's stable first-appearance slot instead of its current array position, so when an earlier duplicate instance unmounts the surviving instance keeps the value's original slot rather than jumping to the end. Adds tests for the slot invariant, the duplicate warning, and strengthens the surviving-metadata assertion to cover both mounted and unmounted states. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

github-actions · 2026-06-16T10:11:25Z

Looks like one of the E2E tests has failed.

You can fix them following these steps:

Check out this branch locally:
```
gh pr checkout 111567
```
Start Claude Code in the repo:
```
claude
```
Run the /fix-e2e-tests skill, passing this PR number:
```
/fix-e2e-tests 111567
```

ciampo

Good to merge once remaining feedback is addressed

ciampo · 2026-06-16T16:29:27Z

+	const steps = useMemo( () => {
+		const order = slotOrder.current;
+		const canonical = new Map< string, T >();
+		for ( const inst of instances ) {
+			if ( ! canonical.has( inst.meta.value ) ) {
+				canonical.set( inst.meta.value, inst.meta );
+			}
+		}
+		return [ ...canonical.values() ].sort(
+			( a, b ) => ( order.get( a.value ) ?? 0 ) - ( order.get( b.value ) ?? 0 )
+		);
+	}, [ instances ] );


Nit:

This memo reads slotOrder.current during render but only depends on [ instances ].

It happens to work because every slot mutation is paired with a setInstances, so the ref is always current by the time the memo reruns. Still, reading a mutable ref mid-render isn't pure and could bite us under concurrent rendering. Not worth blocking over — duplicates are already an error state — but if we want it airtight, folding the slot into the instances state ({ id, slot, meta }) and sorting on inst.slot drops the ref read entirely.

Good call, done. Folded the slot into the instances state ({ id, slot, meta }) and the memo now sorts on inst.slot, so it no longer reads slotOrder.current mid-render. The ref is still used inside registerStep (outside render), so the derivation is now pure. Slot-pinning behavior is unchanged and still covered by the existing tests.

Fold the per-value slot into the instances state and sort on it, so the steps memo no longer reads the slotOrder ref mid-render — keeping the derivation pure under concurrent rendering. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

bogiii self-assigned this Jun 11, 2026

bogiii and others added 3 commits June 11, 2026 22:49

ui: Add changelog entry for Stepper duplicate-value warning fix

a38140b

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ui: Drop changelog entry for unreleased Stepper fix

02bc01c

The Stepper component is still unreleased, so a bug fix to it needs no changelog entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

bogiii force-pushed the fix/stepper-duplicate-step-warning branch from 60b235b to 02bc01c Compare June 11, 2026 20:58

bogiii requested a review from Copilot June 11, 2026 21:32

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 11, 2026

bogiii marked this pull request as ready for review June 11, 2026 21:32

Copilot started reviewing on behalf of bogiii June 11, 2026 21:32

View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread packages/ui/src/stepper/use-step-registration.ts Outdated

Comment thread packages/ui/src/stepper/test/index.tsx

bogiii marked this pull request as draft June 11, 2026 21:44

bogiii requested a review from Copilot June 11, 2026 21:49

Copilot started reviewing on behalf of bogiii June 11, 2026 21:49

View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

bogiii mentioned this pull request Jun 11, 2026

feat(ui): add Stepper component to @automattic/ui #111036

Merged

3 of 10 tasks

bogiii requested a review from ciampo June 11, 2026 21:59

bogiii marked this pull request as ready for review June 11, 2026 22:00

bogiii marked this pull request as draft June 12, 2026 10:15

bogiii marked this pull request as ready for review June 12, 2026 10:29

ciampo reviewed Jun 15, 2026

View reviewed changes

Comment thread packages/ui/src/stepper/use-step-registration.ts

Comment thread packages/ui/src/stepper/test/use-step-registration.tsx Outdated

bogiii and others added 2 commits June 15, 2026 23:24

Address PR review feedback (#111567)

b32b63f

- Document the stale-metadata limitation when duplicate step values overlap - Rename use-step-registration test to .ts (no JSX) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

bogiii marked this pull request as draft June 15, 2026 22:19

bogiii requested a review from ciampo June 16, 2026 10:08

bogiii marked this pull request as ready for review June 16, 2026 10:09

ciampo approved these changes Jun 16, 2026

View reviewed changes

bogiii merged commit 282d0d5 into trunk Jun 16, 2026
19 checks passed

bogiii deleted the fix/stepper-duplicate-step-warning branch June 16, 2026 22:15

github-actions Bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 16, 2026

Navigation Menu

Repository navigation

ui: Stepper warns about duplicate step values at registration time - #111567

ui: Stepper warns about duplicate step values at registration time#111567
bogiii merged 9 commits into
trunkfrom
fix/stepper-duplicate-step-warning

bogiii commented Jun 11, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 11, 2026 •

edited by matticbot

Loading

Uh oh!

matticbot commented Jun 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

ciampo left a comment

Uh oh!

ciampo Jun 16, 2026

Uh oh!

bogiii Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

bogiii commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

Uh oh!

github-actions Bot commented Jun 11, 2026 • edited by matticbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matticbot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

ciampo left a comment

Choose a reason for hiding this comment

Uh oh!

ciampo Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

bogiii Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bogiii commented Jun 11, 2026 •

edited

Loading

github-actions Bot commented Jun 11, 2026 •

edited by matticbot

Loading

matticbot commented Jun 11, 2026 •

edited

Loading