fix(server-examples): compile at Handsontable 18.1.0 and refresh locks (DEV-2731) - #294
Merged
Merged
Conversation
…s (DEV-2731) 18.1.0 rewrote ColumnSettings from Omit<GridSettings, 'data'> to Omit<RemoveIndexSignature<GridSettings>, 'data'>. Under 18.0.0 that Omit collapsed against GridSettings's [key: string]: any index signature, so every named column option resolved to any. Now they are real types, and an unannotated settings holder widens its literals past them. Angular: annotate the holder as GridSettings, mirroring the examples/ fix in DEV-2727 (PR #281). Only columns[] is actually checked — the wrapper's own GridSettings still extends a plain Omit over the index signature, so top-level options remain any. The single error was columns[].dateFormat widening to string. React: the {...(settings as any)} spread was hiding the same defect rather than avoiding it. Annotate the memo as HotTableProps and drop the cast. HotTableProps maps over GridSettings and does drop the index signature, so top-level options are checked there — which surfaced beforeRowsMutation's narrowed 'create' | 'update' | 'remove' parameter being rejected contravariantly against the declared (operation: string, ...). Widened to string in all 14 TS frontends; the bodies only test operation === 'remove', so this is behavior-neutral. No config values changed: the dateFormat objects were already valid Intl literals, so the fix is entirely type-level and no runtime behavior moves. The vestigial dropdownMenu `as any` casts are removed, verified by build. All 21 package-lock.json files refreshed 18.0.0 -> 18.1.0 via `npm update <pkgs> --package-lock-only`. package.json ranges are untouched: ^18.0.0 already admits 18.1.0, and the lock was what pinned these back. Adds .github/workflows/server-examples-build.yml, since nothing in CI built this tree — which is how both this and DEV-2727 shipped unnoticed. A deterministic job guards source edits, and a weekly canary installs handsontable@latest so the next breaking release is caught on the day it ships rather than at the next lock refresh. Verified: npm ci + npm run build across all 21 frontends, raw (not via rtk, whose filters have previously fabricated a pass summary over real TS errors). 21/21 green; both express/server and nestjs/server have no build script. Negative control recorded red before the fix, and the canary's own step body proven to exit 1 against pre-fix source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3aa26d2. Configure here.
Actions runs `bash -e -o pipefail`, so the while loop's exit status is that of its final iteration. With `node ... && echo`, a project that sorts last and legitimately has no build script short-circuits the `&&`, leaving the loop — and therefore the whole pipeline and the assignment — non-zero. Green today only because symfony/frontend-react happens to sort last and has a build script. Adding a backend that sorts after it would turn a planned skip into a red workflow with no visible cause. Reproduced with a trailing skip under `bash -e -o pipefail` (exit 1), and confirmed the `if` form exits 0 while still discovering all 21 projects. Reported by Cursor Bugbot on #294. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

Fixes DEV-2731. Same defect class as DEV-2727 (#281 / #282), which fixed
examples/and deliberately leftserver-examples/out of scope.The break
18.1.0 rewrote
ColumnSettingsfromOmit<GridSettings, 'data'>toOmit<RemoveIndexSignature<GridSettings>, 'data'>. Under 18.0.0 thatOmitcollapsed againstGridSettings's[key: string]: any, so every named column option resolved toany. Now they are real types, and an unannotated settings holder widens its literals past them.Two coupled problems: the frontends do not compile at 18.1.0, and all 21 are still held at 18.0.0 by their committed lock (the
^18.0.0range already admits 18.1.0). Sonpm ciis green today and the break lands the moment anyone refreshes a lock. Fixing and bumping therefore had to land together — splitting them would leave a commit where the locks are refreshed and 7 Angular projects do not compile, which nothing in CI would catch.What the builds actually showed
Measured with raw
npm ci && npm run buildper project, asserting on exit codes.dateFormat), 4 greenas any21/21 pass.
express/serverandnestjs/serverare excluded: no build script, no Handsontable dependency.Three corrections to the ticket, all from builds rather than reading:
It is 21 frontends, not 23. The ticket counted the two backend
package.jsonfiles.The Angular check surface is
columns[]and nothing else. The wrapper defines its owninterface GridSettings extends Omit<Handsontable.GridSettings, 'columns' | 'data'>, and that plainOmitstill collapses against the index signature — so every top-level option staysany. Onlycolumnsis redeclared, and onlyColumnSettingsreceivedRemoveIndexSignatureat 18.1.0. That is why 4 of the 7 Angular projects were never broken, and why annotating the holder surfaced nothing beyondcolumns[].dateFormat.The React frontends were not safe. The ticket lists them as shielded by
{...(settings as any)}. That cast was hiding the same defect, not avoiding it:const settings = useMemo(() => ({...}))gets no contextual typing, so the literal widens exactly like the Angular holder, andtsc -balready runs in their build.The fix
Angular — annotate the holder as
GridSettings, mirroring #281's idiom (import { HotTableModule, type GridSettings }). One error, in the 3 projects carrying an IntldateFormatobject.React — annotate the memo as
HotTablePropsand drop the cast.HotTableProps, notGridSettings:HotTabletakes settings as individual props, andHotTableProps extends ReplaceRenderersEditors<GridSettings>maps over the type and does drop the index signature. So top-level options are genuinely checked there — which surfaced exactly one error per file:beforeRowsMutation's narrowed'create' | 'update' | 'remove'parameter rejected contravariantly against the declared(operation: string, ...). Widened tostring; every body only testsoperation === 'remove', so this is behavior-neutral.No config value changed. The
dateFormatobjects were already validIntlliterals ('numeric','2-digit') — the error was pure literal widening, cured by the annotation. The diff is entirely type-level and no runtime behavior moves in any of the 21 projects. Neither judgment call this could have required (an option silently ignored for years, anIntlvalue that needed correcting) actually arose.The vestigial
dropdownMenu: [...] as anycasts are removed — verified by build in both flavours, including React where top-level is checked.Locks — all 21 refreshed 18.0.0 → 18.1.0 via
npm update <pkgs> --package-lock-only.package.jsonis untouched:^18.0.0already admits 18.1.0, and the lock was what pinned these back. (npm install --package-lock-onlypreserves an already-satisfying resolution — the npm counterpart of the pnpm trap #281 documented.)CI coverage
Nothing in CI built this tree, which is how both this and DEV-2727 shipped unnoticed. Adds
.github/workflows/server-examples-build.yml:build— deterministic, per-project matrix,npm ci. Guards source edits. By construction it can never see a new upstream release.build-latest— weekly canary, installshandsontable@latestbefore building. This is the job that would have caught 18.1.0 the day it shipped. Expected to go red on a breaking release; that is the signal.The matrix is discovered at run time, so adding a backend or frontend needs no edit here.
npm lsasserts rather than reports (exit 1 on a tree that does not satisfypackage.json) — a stale or hand-edited lock is the one failurebuildcannot otherwise see.Two things a reviewer should know
The Angular
beforeRowsMutationwidening is forward-looking hardening, not part of the fix. Those 7 files compile identically before and after, because the property isanythere. I widened it anyway so the paired demos stay identical and so a future wrapper release applyingRemoveIndexSignaturetoGridSettings— asColumnSettingsalready got — cannot re-break them. Do not hunt for the Angular error it addresses; there isn't one yet.The workflow has never run under Actions. Two things only a real run proves:
fromJSONexpanding into a 21-entry matrix, and per-projectcache-dependency-pathresolving. The canary's step body is proven able to fail — I ran it verbatim against pre-fix source and got exit 1. Worth noting two earlier attempts at that proof came back falsely green (a latent project, then reverted React whoseas anyshield returns), which is exactly why the negative control matters.Verification
npm ci && npm run buildacross all 21 frontends: 21/21 green. Run raw, not throughrtk— its filters have previously fabricated a pass summary over real TypeScript errors.express/client-angular,TS2322 … 'dateFormat.year' … Type 'string' is not assignable to type '"numeric" | "2-digit" | undefined').package.jsondrift.Master only.
server-examples/onprod-examples/18is already a commit behind (DEV-2548 was never ported there), so that tree is master-only by convention; reconciling it deserves its own ticket.Note
Low Risk
Changes are limited to example apps, lockfile bumps, and CI; runtime behavior is described as unchanged aside from compiling against tighter public types.
Overview
Adds CI for
server-examples/viaserver-examples-build.yml: a runtime-discovered matrix runsnpm ciandnpm run buildon every frontend with a lockfile and build script; PRs get that deterministic job only, while weekly/dispatchbuild-latestreinstallshandsontable@latest(and the framework wrapper) to catch upstream type breaks.Handsontable 18.1.0 is applied across all 21 example frontends by refreshing
package-lock.json(core and@handsontable/*wrappers);package.jsonranges are unchanged.Angular demos type the table config as
GridSettingsso stricterColumnSettingsin 18.1.0 can compile;beforeRowsMutationusesoperation: stringto match the hook signature, and vestigialdropdownMenuas anycasts are removed.React demos use
useMemo<HotTableProps>, spread props intoHotTablewithoutas any, and apply the samebeforeRowsMutationanddropdownMenutyping cleanup.Reviewed by Cursor Bugbot for commit 51cea5d. Bugbot is set up for automated code reviews on this repo. Configure here.