Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-timeline): declare who owns the bare timeline key, and pin it against a reorder - #6417
Merged
os-support-ai merged 1 commit intoAug 25, 2026
Conversation
`plugin-timeline:timeline` (presentational, `src/renderer.tsx`) and `view:timeline` (object-bound, `src/index.tsx`) both claimed the bare `timeline` fallback, so the winner was whichever module evaluated last. `src/index.tsx` re-exports `./renderer` before its own `import`, so the object-bound renderer won by accident. Register the presentational renderer with `skipFallback: true` — the remedy the registry's collision guard names — so only `view:timeline` claims the bare key, in any evaluation order. Resolution is unchanged today; it is now decided rather than inherited. Add `src/__tests__/timeline-bare-key-ownership.test.ts`, which replays both registrations' real declared metadata into a fresh `Registry` in both orders, so a reorder reddens instead of silently swapping renderers. Ref objectui#6353
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 25, 2026
os-support-ai
marked this pull request as ready for review
August 25, 2026 23:42
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-6353-timeline-double-registration
branch
August 25, 2026 23:54
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#6353
packages/plugin-timelineregisters the same short name twice, and until now both registrations also claimed the baretimelinekey, so the winner was whichever module evaluated last.src/renderer.tsx:498TimelineRenderer(presentational)plugin-timelinesrc/index.tsx:366ObjectTimelineRenderer(object-bound)viewsrc/index.tsxre-exports./renderer(line 300) before its ownimport(line 307), so the presentational registration ran first and the object-bound one overwrote it. The outcome was the intended one; the mechanism was module-evaluation order. Reordering those two lines would have handedtype: 'timeline'to the presentational renderer, which reads none of the object-bound keys (object/filter/sort/limit) — an authored timeline would have stopped fetching, with no error and no failing test.The change
The fix —
src/renderer.tsxregisters withskipFallback: true, the remedy the registry's own collision guard names in its warning text (packages/core/src/registry/Registry.ts:246-266). No second mechanism is introduced. Onlyview:timelineclaims the bare key now, in any evaluation order; the presentational renderer stays reachable under its explicitplugin-timeline:timelinekey. Resolution is unchanged today —type: 'timeline'still rendersObjectTimelineRenderer— it is now decided rather than inherited.The pin —
src/__tests__/timeline-bare-key-ownership.test.ts, 5 assertions. It reddens if the declaration is dropped, if a third registration starts claiming the bare key, or if resolution becomes order-dependent again.The load-bearing one is the replay: asserting only today's outcome cannot distinguish declared from happened to be last. So the test reads both registrations' real declared metadata back out of the registry — nothing in it is a hand-copied mirror of the source, so it cannot drift from it — and replays them into a fresh
Registryin both orders:Order-independence is then a property under test rather than a property of the file the test happens to import. The pin also asserts the collision guard stays silent, since that warning is the mechanism this fix uses.
Ghost-assertion check — the pin was proven able to fail
Mutation: delete
skipFallback: truefromsrc/renderer.tsx(the pre-fix condition). Predicted before the run: red. Mutation confirmed on disk before measuring —grep -c 'skipFallback: true'went1 → 0and the blob hash moved off theHEADblob; restore was bygit checkout HEAD -- <abs path>and verified by hash equality plus an emptygit diff HEAD, not by exit code.The prediction was one failure short, and the miss is worth recording. I predicted 3 of 5 red and got 4: the
presentational first — what ../index evaluates todayrow also failed, on the collision-warning assertion (the registry warned that the bare-name fallback was overwritten) rather than on resolution. In that order the pre-fix code resolves baretimelinecorrectly — and still warns, which is precisely the registry telling us the answer was an accident.The one row that stayed green under the mutation is the live-singleton assertion (
resolves bare to the object-bound renderer), because pre-fix the real registry still resolved correctly by evaluation order. That is the concrete demonstration that a live-outcome assertion alone would have been a ghost assertion here: it passes in both worlds. The replay is what carries the pin.Census — report only, not fixed here (#6353's closing question)
Censused the registration call shape, not the key string: every
.register(/.registerLazy(call acrosspackages/**andapps/**(1575 files, 267 literal-key registrations), extracting(key, namespace, skipFallback)and reporting bare keys claimed by more than one namespace. Each hit was then read.False positives, all confirmed by reading:
'${type}'and'button'areRegistry.ts's own JSDoc examples and its deprecation-warning template;'form'/'list'atpackages/react/src/spec-bridge/SpecBridge.ts:20-21are a different registry (SpecBridge's ownMap<string, BridgeFn>), notComponentRegistry. The dynamic-key sites are all deliberate and already guarded —packages/fieldsdrivesskipFallbackoff a declaredFIELD_TYPES_SKIP_FALLBACKset and registers tombstones withskipFallback: true;placeholders.tsx:126guards withif (!ComponentRegistry.get(type)).One real double-claim outside
plugin-timeline, filed as #6416 and deliberately not touched here:packages/plugin-reportregistersreport/spec-report/report-viewerunder namespacereport, whileapps/console/src/register-plugins.ts:135and its variant loop declare the lazy stubs underplugin-report. Two consequences — barereportis double-claimed order-dependently (this card's shape), and theplugin-report:*keys are never satisfied by a real registration, yetpackages/cli/src/utils/known-schema-types.ts:345-347whitelists all three. Which spelling is canonical is a judgement call with consumer-visible consequences either way, so it is filed for triage rather than decided in this PR.Verification
All of the below ran on the exact tree of the final commit,
git rev-parse --short HEAD=48da81623(git diff HEADempty at the time of each run). Heavy runs went through the container's shared verify lock.pnpm exec vitest run packages/plugin-timeline/src/(canonical root invocation)Test Files 10 passed (10)·Tests 73 passed (73)pnpm --filter @object-ui/plugin-timeline type-checktsc --noEmit && tsc -p tsconfig.test.json, so not a zero-match scriptpnpm --filter '@object-ui/plugin-timeline^...' build(dependency closure, built first)node scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjs/check-changeset-fixed.mjsnode scripts/check-control-bytes.mjsnode scripts/check-doc-component-types.mjstsconfig.test.jsonincludessrc/**/*.test.ts, so the new test file is genuinely in the type-check program rather than excluded from it.Lint was narrowed, and here is why the narrowing is a measurement rather than a skip. Repo-wide
pnpm lintis CI's run. Locally I linted the diff:pnpm exec eslint <the two changed source files> --format jsonreports 2 files, 0 errors, 15 warnings. All 15 warnings arereact-refresh/only-export-componentsand@typescript-eslint/no-explicit-anyat lines ≤ 463, i.e. entirely above my edit region (498+) — pre-existing, none introduced here. The population is eslint's own: its config lints**/*.{ts,tsx}, which covers exactly those two of my three changed files (the third is.changeset/6353-*.md). And the narrowing cannot hide anything, because type-aware linting is not enabled —eslint.config.jsextendstseslint.configs.recommendedand contains zero occurrences ofprojectService,parserOptions.projectorrecommendedTypeChecked— so no rule reads across file boundaries and this diff cannot move the verdict on any file it did not touch.A changeset is included (
patch,@object-ui/plugin-timeline); nomajor, per the fixed-group rule.Generated by Claude Code
Generated by Claude Code