Repository navigation
fix(router): each unit test starts with no suspense-audit records, and a violation fails it (rango-t226) - #1318
Merged
Merged
Conversation
… I4 remount, and an audit violation fails its test (rango-t226, red)
…o suspense-audit records, and a violation fails its test (rango-t226)
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
devcorner: rango-t226 · session rango-s448
The router's unit suite now fails a test when the suspense audit reports a violation in it. Before this PR, the suite printed 368 audit reports at router 0.30.0 that no test failed on. Each test, and each
renderRouteroot, now starts with no audit records.Problem
The suspense audit (
src/suspense-audit.ts) runs only whereINTERNAL_RANGO_SUSPENSE_AUDITis set: the router's own unit suite and the e2e servers. Consumers never run it, so nothing changes for them.The audit keeps its records in module state, and vitest keeps one module per file, not per test. So a root that one test unmounted stayed in the records. The next test's first mount of the same outlet then read as an I4 remount.
Nobody saw it. The default reporter drops console output, and no test failed on a report. On main,
vitest run --reporter=verboseprinted 368 reports:store.emitUpdatewithout naming the cause, as production's emitters doA real violation in a unit test was lost among them. The lock also surfaced 2 I4 and 4 I5 in
action-loader-error-in-place.test.tsx, which mocksconsole.errorand so hid even the printed warning.Before and after, for a test in this repo
Before: a test that leaves the audit in a bad state passes, and the next test inherits the records.
After: the same test fails with the report, and names the cause as production's emitters do.
The failure message, after the audit's queued judgments have run:
A test that hits a known router defect opts out by name. The opt-out fails once the defect is fixed, so it does not outlive its ticket:
Semantics
renderRouteroot in one test, aftercleanup()suspense-audit.test.tsx)renderRoutetestsforgetSuspenseAudit()is a no-op without the flag__rangoSuspenseAuditCode changes
src/suspense-audit.ts:forgetSuspenseAudit()re-creates every record, including theWeakMap/WeakSetones, which have noclear(). It no longer creates a report when the audit is off.getReport()lists each loaded copy of the module onwindow.__rangoSuspenseAudits, as aSuspenseAuditInstance:violations(),forget()andsettle(). One file,segment-system.test.tsx, re-imports the module aftervi.resetModules(). That file then holds two copies, and its static imports use the older one, not the one on__rangoSuspenseAudit.eventLine()formats an event for both the console line and the failure message.src/testing/render-route.tsx: callsforgetSuspenseAudit()before it builds its first tree.src/__tests__/kit/suspense-audit-lock.ts(new; a vitest setup file invitest.config.ts):vi.stubEnv, and that broke 3logging.test.tstests.auditTreeCause, andauditTreeUpdatewhere no provider does it:action-loader-error-in-placenavigation-adoption-loadingnavigation-scroll-slotlocation-state-hydrationnavigation-provider-hydrationnavigation-provider-cached-handlesdocs/internal/suspense-contract.mdhas a new section, "The unit-test lock";docs/internal/feature-file-map.mdis updated.Test plan
Red before green. On the test commit (
12fbcfd3a), 4 tests fail; on the fix commit, all pass:render-route-audit-lock.test.tsx:it.fails;uncaused === 0.render-route-audit-reset.test.tsx: a root in the next test, and a second root in one test, mount the same outlets with no I4.With the
renderRouteforget removed, the lock fails the two-roots test withI4 remounts: 1. This showssettle()waits for the audit's deferred remount judgment.Router unit suite: 9635 passed and 1 expected fail, across 585 files. With
--reporter=verboseit prints 1 audit report, down from 368: the I7 that is allowed under rango-t227.pnpm run typecheck,pnpm run test:unit:all,pnpm run lint,pnpm run format.Every
check:*script, includingcheck:e2e-parity --strict,check:suspense-contractandcheck:internals.check:bundle-guards: pass; the router chunk is 52279 B gzip, unchanged from main, under the 52352 B cap.node --teston thetoolstests: 63/63.Targeted e2e,
suspense-casesandheld-boundary, run before the final review cleanup. That cleanup changed no console output.CI's full e2e run on this head.
Notes
src/testing/__tests__/navigation-adoption-loading.test.tsxis ontools/hook-lock.json. The edit is test-only: each of the 3 directstore.emitUpdatecalls now names its cause first. No hook's commit behavior changes. Thehook-lockcheck needs the labelhook-lock: approved. Hooks design: rango-t84, the hooks review this belongs to.window.__rangoSuspenseAudits, one entry per copy. Only the lock reads it. It exists becausevi.resetModules()loads a second copy, and the lock must reset and check both.redirect()dropsStreamedLoaderErrorBoundary, so the wrapper chain drifts (I5). 4 tests inaction-loader-error-in-place.test.tsx.renderSegmentswritesloaderIdson a slot segment already handed to React (I7). 1 test insegment-system.test.tsx.auditTreeCausewith an emit by hand. They mirror production's emitters, which make the same two calls, and their shapes differ (emitUpdate,setTreewithauditTreeUpdate,commitInTransition). A shared helper would cover only part of them.Practices
pnpm run lint,pnpm run formatand everycheck:*script passed on the head.settle()in place of a fixed microtask count, and the ungatedrenderRouteforget.window. A reset on root unmount would not clear the counters, and would hide a whole-tree remount.suspense-contract.md, "The unit-test lock";feature-file-map.md.