fix(plugin-timeline): judge a gantt Date by its [[DateValue]] slot, not its prototype - #7037
Merged
Merged
Conversation
…ot its prototype `isGanttDateType` used `value instanceof Date`, which answers "does this inherit from `Date.prototype`" rather than "is this a Date". A slot-less impostor passed the gate, reached `new Date(value)`, ran ToPrimitive and threw uncaught mid-render — a blank screen where objectui#6781's named diagnostic belongs. Both Date tests in this file now ask a total brand test built on the builtin `Date.prototype.getTime` invoked with `.call`: it reads the receiver's `[[DateValue]]` slot and nothing else, so no author getter runs, no `Symbol.toStringTag` is consulted and no proxy trap fires. The speller's branch selector had to move with the gate: with the gate fixed and `spellGanttDateValue` still selecting on `instanceof Date`, the impostor is refused and then crashes inside `Date.prototype.toString.call` while being named — the crash relocates instead of closing. Measured both ways; no spelling changes for any value that reaches the speller today. Adds a pinned adversarial input set so the totality is exercised rather than asserted for a fifth time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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 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
1 participant
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 #7027
The decision: route 1, with a brand test that is neither of the two the card proposed
The card offered two routes and triage judged route 2 (narrow the stated claim) the likely answer, on the ground that route 1 "is not free". I measured both proposed route-1 spellings, and both are indeed wrong — but the conclusion that follows is not route 2. It is that route 1 has a third implementation the card did not consider, and that implementation is free.
What ships: one module-local predicate, used at both places in
renderer.tsxthat ask "is this a Date".It invokes the builtin with
.call. That reads the receiver's[[DateValue]]internal slot and nothing else: no property is fetched offvalue, so no author getter runs, noSymbol.toStringTagis consulted, no proxy trap fires, and a subclass cannot hijack it. The slot is not observable any other way — the language exposes that bit only by throwing when it is absent — so thecatchis the read, not error handling draped over a fallible operation.Why not route 2
Route 2 asks the docblock to state which inputs the totality claim covers and why the rest cannot arrive. That is a claim about reachability, which is a global property of the whole repo — every adapter, every test helper, every deserializer anyone writes later. Route 1's property is local: "this predicate cannot throw", checkable in one file forever and pinnable by a test. Route 2 replaces a false local claim with a true-today global one, which is a strictly harder invariant to keep and exactly the shape that has been falsified three times on this path already.
There is also a mechanical reason route 2 could not have carried the deliverable: under route 2
Object.create(Date.prototype)still crashes, so the pinned adversarial set could not assert a named diagnostic for it.Why not either brand test the card suggested — both measured, both refuted, both pinned red
Object.prototype.toString.call(value) === '[object Date]'{ get [Symbol.toStringTag]() { throw } }Object.prototype.toStringperformsGet(O, @@toStringTag)unconditionally (ES2015 19.1.3.6 step 16), even once the builtin tag is decided. Trades an impostor crash for a getter crash — the same input #6907 measured crashing the speller.Number.isFinite(value.getTime())class X extends Date { getTime() { throw } }getTime. That subclass is a real Date —super()gave it the slot, so #6781's accept set contains it and the chart must draw — and this spelling would refuse it by dying on it. Worse than the bug: it breaks a value that is not even wrong.Both rows are in the pinned suite. A later "simplification" into either spelling fails there rather than in a fifth card.
The four-facet analysis
Real business need (实际业务需求). Low, and stated honestly: no authored document reaches this. I verified the card's reachability analysis rather than accepting it (see below) and it holds. The need this serves is not an author's — it is the next engineer's. #6905's totality argument is written out loud in the code and is load-bearing for the order of two operations; a reader who trusts it and reorders them ships a crash. The cost of being right here is 8 lines of code.
Long-term soundness for this project (长远合理性). This is the facet that decides it. The path now carries a local, checkable property instead of a global, unverifiable one, and the property is exercised rather than asserted. Contract-first is respected: nothing became tolerant, no consumer gained a fallback, and the producer-side contract (#6781's accept set) is untouched — the change makes an existing refusal arrive instead of crashing on the way.
Making AI-written code hard to get wrong (防 AI 写错). The strongest argument for route 1 over route 2. A narrowed prose claim is text that a later agent can silently re-widen, and nothing goes red. A total predicate plus a pinned adversarial set is structural: the two obvious "cleanups" of
isDateeach have a dedicated failing test with the reason written on it. The docblock now also says, in the imperative, what to do with the next gap — add the row to that file, do not write a fifth sentence.Startup scope discipline (创业阶段不扩散). Net +8 lines of code, one module-local helper, zero exports, no new capability, no spec key, no new i18n key, no locale packs opened, no accept-set adjudication. The suite is the bulk of the diff and it is the deliverable the card asked for. I deliberately did not fix the one remaining non-total operation I found on this path (filed as #7036 instead) precisely on this facet — see below.
The measurement: crash reproduced on base first, then closed
The pinned suite was written against unmodified
7fc5c3c12and run there beforerenderer.tsxwas touched. Three rows failed, each an uncaughtTypeErrormid-render:The third row is a finding of its own:
instanceofis not total on its own terms — it walks[[GetPrototypeOf]], so a proxy that traps it crashes inside the operator. That was a live crash onmainand the card did not know about it.After the fix, at
e44f8ad82:Test Files 16 passed (16) · Tests 204 passed (204)across the wholeplugin-timelinesuite, so #6759 / #6770 / #6781 / #6907's pinned spellings and the inverted-range message are all still green, unchanged.The pinned adversarial input set
packages/plugin-timeline/src/__tests__/timeline-gantt-date-brand-7027.test.tsx. Every input reaches exactly one of ACCEPTED (it is a real Date, the chart draws) or NAMED (refused through the single existing diagnostic, with the authored path and a spelling) — and never THREW.getTimesubclass are controls that must be accepted.7fc5c3c12e44f8ad82Object.create(Date.prototype)isUnusableis an objectProxythrowing on every getis an objectProxywith a throwinggetPrototypeOftrapinstanceofis an objectProxythrowing on every getclass X extends Date { getTime() { throw } }{ get [Symbol.toStringTag]() { throw } }new Date('2024-01-01')new Date(NaN)Invalid DateInvalid DateThe accept set does not move — and the one edit outside the gate that was forced
Accept set: unchanged.
isDateis strictly narrower thaninstanceof Dateover the values that reach it, and everything it newly refuses is a value no authored document can carry.new Date(NaN)owns its slot, so it still passes the type gate and is still refused one step later by the parse check, keeping itsInvalid Datespelling — pinned. A compact accept/refuse control row re-assertsstring/ finite number / realDateaccepted andnull/undefined/ boolean refused, exactly where #6781 left them.spellGanttDateValue's branch selector had to move with the gate. The dispatch said that helper is finished and must not be touched. It is finished as a set of branches and I changed none of them — but itsDatebranch was selected byvalue instanceof Date, and that selector has the identical defect. With the gate fixed and the selector left alone, the impostor is refused by the gate and then crashes insideDate.prototype.toString.callwhile being named: the crash relocates one function downstream instead of closing, which is precisely the shape #6907 recorded when it found #6905 had moved the crash rather than removed it.That is not an argument, it is an ablation. Committed the fix first, then reverted only the speller's selector back to
instanceof Date(mutation confirmed on disk by grep counts in both directions plus a blob-hash difference againstHEAD), keeping the gate fixed:Same three tests, same class of crash, new line number. The restore leg was proved by state, not by an exit code: the file's blob hash is byte-identical to
HEAD:packages/plugin-timeline/src/renderer.tsx(648f7784...) andgit diff HEADis empty.No spelling changes for any value that reaches the speller today. The two predicates disagree only on slot-less impostors (which crashed before, so nothing reached the speller) and on cross-realm Dates (which no code path here produces). The full 204-test suite is the control.
Reachability: verified, not accepted — and it holds
The card's analysis is the crux, so I checked it on
7fc5c3c12rather than inheriting it:Object.create(appears nowhere inpackages/*/srcorapps/*/srcoutside comments; every hit is prose about__proto__key hazards. NosetPrototypeOfin source at all.superjson,devalue,flatted, noJSON.parsereviver, no date-revival helper anywhere in source. The only structural cloner isstructuredClone, which produces a realDate(its own slot, receiving realm) — not an impostor.No reachable path found. So the p3 grade stands and the crash is a latent-invariant repair, not a live author-facing bug — which is why the argument above leans on the long-term and anti-AI-error facets rather than on business need.
The pattern this card sits in, and why the suite is the real deliverable
spellGanttDateValuetotal by construction.String#7026) measured that the speller was not total — three inputs crashed the render.Four totality claims, three falsified by the next card's measurement. A fifth prose assertion continues the sequence; an exercised input set ends it. Both docblocks now say so in the imperative, and the pinned suite is where the next gap goes.
Out of scope, filed not fixed
#7036 —
spellGanttDateValue'sArray.isArrayis now the last non-total operation on this path: a revokedProxythrowsTypeError: Cannot perform 'IsArray' on a proxy that has been revoked. Not a regression (it crashed insideinstanceofbefore this PR and insideArray.isArrayafter — same render, different line), not reachable, and #6907 choseArray.isArraydeliberately with a measured argument, so the correct replacement shape is an adjudication rather than a mechanical fix. Recorded rather than swept in. #7036 is not addressed here.Checks
Exit codes captured before any pipe; each verdict quoted from the check's own output.
pnpm vitest run packages/plugin-timeline/src/__tests__/Test Files 16 passed (16) · Tests 204 passed (204)ate44f8ad827fc5c3c12Tests 3 failed | 6 passed (9)— the defectTests 3 failed | 6 passed (9), crash atrenderer.tsx:496pnpm --filter @object-ui/plugin-timeline run type-checktsc --noEmit && tsc -p tsconfig.test.json), after building the dependency closuretsc -p tsconfig.test.json --listFilespnpm exec eslint packages/plugin-timelinecheck:control-bytespnpm check:control-bytesOK (scanned 5837 tracked text file(s))check:vi-mock-specifierspnpm check:vi-mock-specifiersOK (509 carry a mock ...)check:vi-mock-inheritpnpm check:vi-mock-inheritOK (112 call site(s) on @object-ui/react judged, 112 inherit, 0 auto-mocked)check:shell-escape-residuepnpm check:shell-escape-residueOK (0 occurrence(s) outside a fence)changeset:checkpnpm changeset:checkAll workspace packages are in the changeset fixed group·No changeset declares a major bumpDeclared narrowing, with its evidence. Repo-wide
pnpm lintwas not run locally; the affected package was, and the narrowing is measured rather than assumed: (1) the population came from eslint's own config resolution, not from my guess about which files count —pnpm exec eslint packages/plugin-timelineresolved the set itself; (2) the count is read from--format json: 26 files, 0 errors; (3) type-aware linting is not enabled (eslint.config.jsdeclares noprojectServiceand noparserOptions.project), so a file's verdict depends only on its own bytes plus the shared config — and this diff touches 3 files, none of themeslint.config.js, so no untouched file's verdict can move. CI runs the full farm regardless.Not run and not owed: no consumer sweep —
isDateis module-local and unexported, no package boundary or.d.tschanged. No i18n gates — not()call site or key was added or removed; the refusal reusestimeline.gantt.unusableRange.malformedDateand the ten locale packs are untouched.Diff
packages/plugin-timeline/src/renderer.tsx—isDateplus its docblock; bothinstanceof Datecall sites swapped; three docblock passages corrected where they asserted a totality that was not true.packages/plugin-timeline/src/__tests__/timeline-gantt-date-brand-7027.test.tsx— new, the pinned adversarial set..changeset/7027-gantt-date-brand-gate.md— patch to@object-ui/plugin-timeline.Draft on purpose — not marking ready, not merging.
Generated by Claude Code