Skip to content

chore(sync): resolve forward-merge conflict (main into next) - #2981

Merged
mfal merged 16 commits into
nextfrom
sync/main-to-next
Aug 28, 2026
Merged

chore(sync): resolve forward-merge conflict (main into next)#2981
mfal merged 16 commits into
nextfrom
sync/main-to-next

Conversation

@mfal

@mfal mfal commented Aug 28, 2026

Copy link
Copy Markdown
Member

Resolves the forward-merge conflict between main and next (ADR 0004 §4), resolved locally with the merge drivers active via pnpm sync:resolve.

Merge this as a true merge commit — a squash or rebase merge would break the superset invariant (ADR 0004 §1) and is blocked by branch protection on next.

This PR is not only conflict resolution. Beyond the three conflicts it carries a generator fix, a new test, and one regenerated file — two cross-side interactions that only break once both branches are combined. Those are the parts worth reviewing.

The three conflicts

apps/docs/src/lib/PropertiesTables/lib/loadProperties.ts — a real semantic conflict. next moved the properties table onto the generated component index (#2897); main taught it to hide @deprecatedValues (#2960, on top of #2961). Kept next's data source and ported main's value hiding onto the new prop shape: name comes from the map key, type is a plain string, description is optional.

Dropped the top-level undefined/null filter from the docs side — the index generator already strips them in dev/component-index/filterProps.ts, so keeping it here would duplicate the rule. The docs side now only removes deprecated values.

docs/adr/0004-forward-merge-main-into-next.md (§3) — took main's passage verbatim. It already contains both changes: the publish-next.ymlpublish.yml rename (#2968) and the version-consistency-guard paragraph (#2958).

pnpm-lock.yaml — kept next's, verified rather than assumed. Zero entries exist only on main (next already carried the same Dependabot bumps), and pnpm install reports it up to date against every merged manifest.

Two cross-side interactions

Neither is in a conflicted file. Both are silent — nothing in either branch's own CI run would have caught them.

1. Button's color came out of the merge flagged deprecated as a whole. main's new @deprecatedValues tag contains the substring next's index generator matched to set deprecated: true:

if (prop.description.includes("@deprecated")) {   // matches "@deprecatedValues accent"

dev/component-index/buildComponentIndex.ts now uses main's /@deprecated(?!\w)/, which was introduced in #2960 for exactly this distinction. Without it, #2960's "nothing about the prop itself changes" stops holding — color keeps working and still warns via useWarnDeprecation, but the docs would have advertised the whole prop as deprecated. A regression test covers it (verified to fail on the old code).

2. RemoteRatingSegmentElement carried a render remote property. main put react-aria's render on the generator's global ignore list (#2966) after next had generated RatingSegment (#2863), so the merged generator output differs from what is committed. Regenerated.

This is not cosmetic: render returns rendered output the host would have to call, and one such property drops the entire mutation batch over postMessage — the bug #2966 fixed for Table. It would also have failed CI's "Check all generated code is committed" gate.

Verification

Check Result
pnpm lint (eslint + stylelint + format:check) exit 0 — 0 errors, warnings all pre-existing
node .github/scripts/version-consistency-guard.mjs 16 managed packages at 1.1.0-next.0
node --test .github/scripts/*.test.mjs 38/38
unit tests (components, docs, remote-*) 445 passed
test:compile components, tsc --noEmit apps/docs clean
pnpm build + git diff clean after the one regenerated file

End-to-end against the real generated index, Button's color reads … | "secondary" with accent gone, deprecated: false, and the tag stripped from the description. ColumnLayout's s still reads (number | null)[], so #2961's nested-null fix survives.

Carries the verify-only run-visual-tests label: this is the first time main's Table and FileField fixes render alongside next's new RatingSegment, and no prior PR's visual run covered that combination.

Escalation: #2963 — closed automatically once next contains main again.

The red run-visual-tests check is inherited from main

Do not add update-screenshots to this PR. All four required checks (main, Conventional PR title, Routing, Version contract) pass; run-visual-tests is not required.

Two Remote/firefox cases in List.browser.test.tsx fail. Both already fail on main's tip — verified by running the same test on each parent commit in a clean worktree:

Test (Remote, firefox) main @ 83d6086 next @ b171f12 this PR
List items ✗ 782 px (0.01) ✗ 782 px (0.01) ✗ 782 px (0.01)
List date range filter ✗ 529 px (0.01) ✗ (Linux only)
List table 95158 px (0.11) fixed here

The merge does not cause either failure, and it repairs List table (Remote), which main's #2966 fixes and next was still failing.

The List items diff is the focus ring missing on the checked filter item in the dropdown — 782 px is the ring, not noise. It comes with an unhandled error that reproduces on both parents:

TypeError: listener is not a function
  at eventListenerCallbackWrapper (@mittwald/remote-dom-react/host/hooks/props-for-element.mjs:54)
  at onChange (packages/components/src/lib/remote/useControlledHostValueProps.ts:34)

The host hands down an onChange that is truthy but not a function, so the onChangeFromProps?.(v) guard does not catch it. That is a real bug on both release lines and wants its own fix — baking the current rendering into the baselines would entrench it.

Nothing here blocks this PR; the visual suite only ran because the label was applied deliberately, and the workflow removed it again after the run.

maaaathis and others added 16 commits August 27, 2026 09:55
)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Three conflicts, plus two cross-side interactions the merge creates.

`loadProperties.ts` — `next` moved the properties table onto the generated
component index (#2897); `main` taught it to hide `@deprecatedValues` and to
tell that tag apart from a prop-level `@deprecated` (#2960, #2961). Kept
`next`'s data source and ported `main`'s value hiding onto the new prop shape
(key-based name, `type` as a string, optional `description`). Top-level
`undefined`/`null` are not filtered here any more — the index generator already
strips them, so the docs side only removes deprecated values.

`buildComponentIndex.ts` — `main`'s new `@deprecatedValues` tag contains the
substring `next`'s generator matched to set `deprecated: true`, so `Button`'s
`color` came out of the merge flagged deprecated as a whole. Uses `main`'s
`/@deprecated(?!\w)/` now; regression test added.

`RemoteRatingSegmentElement.ts` — `main` put react-aria's `render` on the
generator's global ignore list (#2966) after `next` had generated
`RatingSegment` (#2863). Regenerated, which drops the `render` property that
would have broken the whole mutation batch in remote.

ADR 0004 §3 — took `main`'s passage: it already carries both the
`publish-next.yml` → `publish.yml` rename (#2968) and the version-consistency
guard paragraph (#2958).

`pnpm-lock.yaml` — kept `next`'s. It is a strict superset of `main`'s (zero
entries only on `main`; `next` already had the Dependabot bumps), and
`pnpm install` reports it up to date against every merged manifest.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal
mfal requested a review from a team August 28, 2026 07:10
@mfal mfal added sync Forward-merge / promotion sync automated Opened by automation run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch labels Aug 28, 2026
@github-actions github-actions Bot removed the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

Preview environments are ready:

Type URL
docs pr-2981.docs.review.flow-components.de
storybook pr-2981.storybook.review.flow-components.de

Images:

  • docs: ghcr.io/mittwald/flow/docs:pr-2981
  • storybook: ghcr.io/mittwald/flow/storybook:pr-2981

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

❌ Visual Regression Tests Failed

The visual test step did not pass. If snapshots differ from the committed baselines, download the visual-diffs artifact from this run to inspect the actual/diff images (the artifact is absent if the run failed before comparison, e.g. during install or browser setup).

If the differences are intentional, update the baselines by adding the update-screenshots label to the PR.

Run details

@mfal
mfal enabled auto-merge (squash) August 28, 2026 07:25
@mfal

mfal commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

⚠️ Please do not follow the update-screenshots suggestion above on this PR. It would entrench a real regression.

The run-visual-tests failure is inherited from main, not caused by this merge. Verified by running the same test on each parent commit in a clean worktree:

Test (Remote, firefox) main @ 83d6086 next @ b171f12 this PR
List items ✗ 782 px (0.01) ✗ 782 px (0.01) ✗ 782 px (0.01)
List date range filter ✗ 529 px (0.01) ✗ (Linux only)
List table ✗ 95158 px (0.11) ✓ fixed here

Neither branch touched anything that could affect it — next only changed Rating, main only Button/FileField/List/Table. The merge in fact repairs List table (Remote), which next was still failing until #2966 arrived.

The 1% diff is not noise: the checked filter item in the open dropdown loses its focus ring, and those 782 px are the ring. It comes with an unhandled error that reproduces on both parents:

TypeError: listener is not a function
  at eventListenerCallbackWrapper (@mittwald/remote-dom-react/…/host/hooks/props-for-element.mjs:54)
  at onChange (packages/components/src/lib/remote/useControlledHostValueProps.ts:34)

Line 34 is onChangeFromProps?.(v) — the host passes an onChange that is truthy but not a function, so the optional-call guard does not catch it. That wants its own fix: on the main line so it forward-merges up.

It went unnoticed because run-visual-tests is an opt-in label, not a required check. All four required checks here (main, Conventional PR title, Routing, Version contract) pass.

🤖 Addressed by Claude Code

@mfal mfal added the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Aug 28, 2026
@github-actions github-actions Bot removed the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Aug 28, 2026
mfal added a commit that referenced this pull request Aug 28, 2026
…ivers it

Takes .github/scripts/version-contract-lib{,.test}.mjs verbatim from `main`,
where #2983 fixed the guard reading a package's first publish as a tightening.

This PR's version-contract check runs the script from its own tree, so it
cannot pass until that fix is present here. The cascade would normally supply
it, but the forward-merge is stuck on a pnpm-lock.yaml conflict and the sync PR
resolving it (#2981) predates the fix and has a red visual suite of its own.

Byte-identical to `main` on purpose: when the cascade does reach `next`, git
sees no difference and there is nothing to reconcile. Nothing is authored here
— the fix itself was reviewed and merged in #2983.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal
mfal merged commit a27c61b into next Aug 28, 2026
26 of 28 checks passed
@mfal
mfal deleted the sync/main-to-next branch August 28, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Opened by automation sync Forward-merge / promotion sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants