[DOM] Keep Fragment sibling positions stable across renders - #37626
KayanoLiam wants to merge 2 commits into
Conversation
|
Hi @KayanoLiam! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Resolve ReactDOMFragmentRefs test insertion conflict by retaining both the sibling alternate regressions and upstream root-container ancestry coverage. Revalidated DOM/reconciler, Fabric, stable-channel compatibility, full lint/format, and DOM/Fabric Flow checks.
|
Merged current main in 366a4f2 and resolved the conflict while preserving both the sibling-position regressions and the upstream ancestor test. GitHub now reports no merge conflict. All rerun local regression/related suites, stable-channel checks, full ESLint/Prettier, and DOM/Fabric Flow checks passed; exact commands and results are in the updated PR description. Upstream CI has not run: Build and Test, Shared Lint, and ESLint Plugin E2E all report |
Summary
Fixes #37610.
FragmentInstance.compareDocumentPosition()can returnIMPLEMENTATION_SPECIFICinstead ofPRECEDINGorFOLLOWINGon alternating renders. The sibling traversal compares Fibers by identity, but DOM nodes and Fragment boundaries can refer to the alternate of the Fiber being visited.Recognize the alternate in both the target and boundary comparisons, as the nearby containment and common-ancestor checks already do. The existing traversal order and boundary checks remain in place, including rejection when DOM order disagrees with the Fiber tree. No public API signatures change.
Merged upstream
main(2b19aecd0e9111b774fad0fad9862e50bcb5bc8a) in366a4f279b4c3b50d8e04905c92bec88e42220f6. Resolved the test insertion conflict by preserving both this PR's three sibling-position regressions and the upstream root-container/ancestor test from #37613. The PR diff remains limited to two files.How did you test this change?
All results below are local, using Node v20.19.0, Yarn 1.22.22, and
yarn install --frozen-lockfile --non-interactive(passed). They were rerun on the merged tree, not copied from the original PR's results.Red/green regression check
Temporarily removed only the four alternate comparisons to restore upstream behavior, then ran:
yarn test ReactDOMFragmentRefs --runInBand --env=development --testNamePattern='sibling positions'yarn test ReactDOMFragmentRefs --runInBand --env=production --testNamePattern='sibling positions'Each baseline run had 2 expected failures and 1 passing control: repeated-render and keyed-reordering checks expected
2/4, but received32; the DOM/Fiber-order disagreement control passed. After restoring the fix, both exact commands passed all 3 selected tests.Related suites and compatibility
yarn test ReactDOMFragmentRefs ReactDOMServerIntegrationFragment ReactFragment ReactTopLevelFragment ReactIncrementalReflection --runInBand --env=developmentyarn test ReactDOMFragmentRefs ReactDOMServerIntegrationFragment ReactFragment ReactTopLevelFragment ReactIncrementalReflection --runInBand --env=productionyarn test ReactDOMFragmentRefs ReactDOMServerIntegrationFragment ReactFragment ReactTopLevelFragment ReactIncrementalReflection --releaseChannel=stable --runInBand --env=developmentyarn test ReactDOMFragmentRefs ReactDOMServerIntegrationFragment ReactFragment ReactTopLevelFragment ReactIncrementalReflection --releaseChannel=stable --runInBand --env=productionyarn test ReactFabric --releaseChannel=xplat --runInBand --env=developmentyarn test ReactFabric --releaseChannel=xplat --runInBand --env=productionnode scripts/tasks/eslint.jsyarn prettier-checknode scripts/tasks/flow-ci.js dom-browsernode scripts/tasks/flow-ci.js dom-nodenode scripts/tasks/flow-ci.js fabricgit diff --check upstream/main...HEADDuring the merge,
git diff --cached --checkrelative to the old PR head reported whitespace in incoming upstream compiler snapshots. Its diagnostics were byte-for-byte identical togit diff --check HEAD upstream/mainbefore the merge commit. Those unrelated upstream files were not edited; the actual PR diff check passes.GitHub CI
For commit
366a4f279b4c3b50d8e04905c92bec88e42220f6, GitHub reports the PR as mergeable (the conflict is resolved), but merge state isblocked.These are not failing test jobs or flaky-test results: the workflows have not executed. Maintainer action is required before upstream CI can be validated; the contributing account has no write/maintain/admin permission on the upstream repository. GitHub CI is not yet verified, and local passes above must not be read as CI passes.
AI assistance: OpenAI Codex assisted with implementation, regression tests, review, conflict resolution, and validation.