-
-
Notifications
You must be signed in to change notification settings - Fork 9k
fix(hydration): don't error if data-allow-mismatch provided for fragment #12505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
51f813c to
ab617d5
Compare
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-ssr
@vue/compiler-sfc
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
|
Thanks for the PR. I've made some minor tweaks 9e90946 |
|
@edison1105 thanks you so much for instant help! I very hope the PR will be merged next release |
|
@edison1105 can you merge this PR, please? |
WalkthroughThe pull request refines hydration error handling by removing test-mode special treatment from mismatch logging, conditionally suppressing mismatch errors when children mismatches are explicitly allowed, and introducing a custom Vitest matcher ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/setup-vitest.ts (1)
8-13: AddtoHaveBeenErroredtoCustomMatchersinterface with correct signatureThe
toHaveBeenErrored(): Rdeclaration is missing thereceivedparameter. According to Vitest's custom matcher API, matchers are functions called with(received, ...expected)where the first argument is the value passed toexpect(...). The correct TypeScript signature should betoHaveBeenErrored(this: CustomMatcherContext): Ror similar to match the documented Vitest pattern. Update the interface to properly reflect that the matcher receives thereceivedvalue as its implicit context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime-core/__tests__/hydration.spec.ts(2 hunks)packages/runtime-core/src/hydration.ts(2 hunks)scripts/setup-vitest.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/setup-vitest.ts (1)
packages/compiler-sfc/src/script/context.ts (1)
error(146-150)
🔇 Additional comments (7)
packages/runtime-core/src/hydration.ts (1)
58-66: Expose hydration mismatch errors in tests while keeping noise minimalRemoving the
__TEST__short‑circuit and relying solely onhasLoggedMismatchErrormeans the production‑level error message is now visible under test, but still only emitted once per process. That lines up nicely with the newtoHaveBeenErroredmatcher and keeps test output controlled.packages/runtime-core/__tests__/hydration.spec.ts (2)
679-680: Assert that Teleport unmount mismatches surface as an errorThis expectation correctly ties the Teleport unmount mismatch to the global
"Hydration completed but contains mismatches."error, matching the new logging semantics and ensuring the regression that originally hid this signal in tests can’t come back.
2475-2478: Confirmdata-allow-mismatch="children"suppresses the global error for fragment under-hydrationThis negative
toHaveBeenErroredassertion nicely covers the fragment “not enough children” case and verifies that, withdata-allow-mismatch="children"on the container, we no longer surface the global hydration‑mismatch error even though the fragment structure is being fixed up.scripts/setup-vitest.ts (4)
103-105: Track a dedicatedconsole.errorspy alongsideconsole.warnDeclaring
let error: MockInstancenext towarnkeeps the spying symmetrical and makes the new matcher and cleanup logic straightforward.
107-114: Spy on and stubconsole.errorinbeforeEachInitializing
error = vi.spyOn(console, 'error')and no-oping its implementation ensures all error logs are captured and won't pollute test output, while still being fully observable via the matcher andafterEachchecks. Vitest guaranteesbeforeEachruns before every test case, so the spy is always initialized before any test logic executes.
135-149: Enforce assertion ofconsole.errorcalls after each testThe
nonAssertedErrorsfilter and subsequent throw mirror the existing warning behavior and enforce test hygiene by surfacing any unexpectedconsole.errorusage as a test failure. This is a recognized best practice in Vitest/Jest test setups—failing on unasserted console calls makes stray errors explicit rather than noisy and ensures tests remain deterministic.Note this assumes the first argument to
console.erroris a string (same assumption as for warnings); any tests logging non-string values directly may need to be updated to wrap them in a message string.
18-37: NewtoHaveBeenErroredmatcher correctly implements Vitest custom matcher signatureThe matcher properly receives
received: stringfromexpect(), searcheserror.mock.calls, marks the substring as asserted, and returns the correct ExpectationResult shape with pass boolean and message function. The implementation follows the same pattern astoHaveBeenWarnedand is appropriate for asserting on specific error messages and catching unasserted errors inafterEach.
Fixes
hydrateFragmentmethod throwshydration mismatcherror despite ofdata-allow-mismatchattribute exists in parent node.fragment not enough childrentest was wrong sinceHydration completed but contains mismatches.error was ignored in test environment and not checked at all__TEST__flag prevented to catch error for testingadded
toHaveBeenErroredmethod that looks same astoHaveBeenWarned, buterrorlevelSummary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.