refactor: extract the canvas package, and make the interact transport injectable - #312
Open
ivanmkc wants to merge 4 commits into
Open
refactor: extract the canvas package, and make the interact transport injectable#312ivanmkc wants to merge 4 commits into
ivanmkc wants to merge 4 commits into
Conversation
First half of the canvas extraction: flow-patch, component-patch and panes-patch have no dependencies outside their own set, so they move first and prove the package wiring before React enters the picture. Their 36 tests pass unchanged in the new home. The viewer now imports them from the package. Four renderer files referenced them as ../../ from client/renderers and were missed by the first pass; the typecheck caught it. Verified: viewer builds, and its suite reports 709 passing — the original 745 minus the 36 that moved. The two failures are pre-existing on Node 25, where a native localStorage shadows happy-dom's; CI pins 20 and 22. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the extraction. renderers/, registry, ansi, theme, interact and css.d move from the viewer client, and flow-geometry, framing-lint and element-density move from the viewer server, together with 25 tests. Adds a node-safe subpath. The barrel exports renderInto, so a server importing it dragged React, Leaflet and their CSS into a Node bundle and esbuild refused on a .png. @ivanmkc/termchart-canvas/lint now carries the patch appliers and the two lints alone, and the viewer server imports that. Two claims in the plan were wrong, both from grep silently skipping a file it treats as binary: element-density is not orphaned (flow-geometry imports it), and three more tests belonged to the canvas rather than the viewer. Verified: canvas 468 passing, viewer 277 passing — 745 together, exactly the count before the move — plus the full viewer e2e suite, which is what proves the lazy per-type chunking still works. The two remaining failures are the pre-existing Node 25 localStorage ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A spike showed that with no server present, ticking a Checklist box appears to succeed: the interact POST 404s and nothing visibly reverts, so the user sees a change that was never saved. That makes this seam a correctness fix rather than a convenience. postInteract now delegates to an installed transport, defaulting to the viewer's existing endpoint so nothing changes there. lifeboard will install one that writes to its local store. Failure handling is unified: a non-2xx, a network error and a throwing transport all drop the echo-guard entry, so the caller's reverting re-render is never mistaken for a self-echo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The i18n test moved here with the renderers, and so did its dependence on a working localStorage. Same setup file as the viewer's, for the same Node 25 reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 2, 2026
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.
The stack
localStoragefor the test suite on Node 25@ivanmkc/termchart-canvas; injectable interact transportEvery branch in the stack was checked out on its own and verified independently:
npm install,tsc --noEmitfor every package,npm testacross the workspace, and the offline e2e where it exists. All nineteen are green — no branch depends on a later one to build or pass. Full viewer e2e (11 suites, 106 assertions) was run on #312 as the behaviour-preservation evidence for the extraction.Stack position: 2 of 10. Base:
lifeboard/00-test-env.Extracts the renderers, registry and board lints out of
packages/viewerinto@ivanmkc/termchart-canvas, so the viewer and a second app render through one implementation rather than two that drift.Behaviour-preserving. The 745 tests that passed before the move pass after it, split 468/277 across the two packages, plus the 3 new tests for the seam below.
Two things worth a reviewer's attention:
The package has two entry points, and the split is load-bearing.
.is the browser barrel;./lintexports only the patch appliers and the two lints. Without it the viewer's server bundle pullsrenderInto→ Leaflet → its CSS → a.png, and esbuild fails with "No loader configured for .png".The interact transport is now injectable. A
Checklisttoggle used to POST the viewer'sinteractendpoint unconditionally; it now goes throughsetInteractTransport. This exists for correctness, not tidiness: an app with no such endpoint gets a silent 404 and the box stays ticked — the user sees a change that was never saved.A note on grep. macOS
greptreatsflow-geometry.tsas binary and returns nothing for it;grep -ais needed. Two "orphaned" files were nearly left behind because of this.Verified: canvas 473, viewer 278, typechecks clean.
🤖 Generated with Claude Code