refactor(core): explicit per-field operation variants + deprecate renderMarkdown#27
Open
LeslieOA wants to merge 1 commit into
Open
refactor(core): explicit per-field operation variants + deprecate renderMarkdown#27LeslieOA wants to merge 1 commit into
LeslieOA wants to merge 1 commit into
Conversation
…derMarkdown Two API hygiene changes bundled. 1. Replace Partial<CanvasNode> with explicit operation variants (closes #5) The previous `updateNodeContent(nodeId, Partial<CanvasNode>)` shape was the last FFI-hostile item from the original audit — Partial<T> has no clean Rust equivalent and would have forced either a stringly-typed HashMap or a sparse-field struct across the JSI/wasm-bindgen boundary. Replaced with six per-field operation variants and matching CanvasState methods: - updateNodeColor(nodeId, color) - updateTextNodeText(nodeId, text) - updateLinkNodeUrl(nodeId, url) - updateFileNode(nodeId, file, subpath) - updateGroupNodeLabel(nodeId, label) - updateGroupNodeBackground(nodeId, background, backgroundStyle) Each Operation variant carries the new value(s) and the prev value(s) for invertOperation, which now has a case per variant. Apply paths verify node type before mutating (silent no-op on missing or mismatched type, consistent with removeNode's `if (!node) return` pattern). Exported `GroupBackgroundStyle = 'cover' | 'ratio' | 'repeat'` so consumers can type-narrow the backgroundStyle parameter without reaching into the GroupNode type. x / y / width / height stay in moveNode / resizeNode where they were before — they're not "content" updates. Breaking change: `updateNodeContent` is removed from the public API. No external consumers in this repo (grep confirmed). Workspace's canvas-ui has the same shape and will need the same migration when it next pulls from upstream. 2. Deprecate renderMarkdown (closes #26) The prop on CanvasView is vestigial — nothing in src/renderer consumes it because TextNodeContent returns null and all text-node bodies render through Skia's paragraphBuilder. The prop has been silently accepted-and-ignored for the entire life of the library. Marked @deprecated in both declaration sites (CanvasView.tsx, CanvasContext.tsx) with a JSDoc explaining the situation and pointing at the actual markdown-rendering pathway. README's "What's inside" section gains a "Markdown rendering" subsection enumerating what's actually supported (inline marks, blocks, GFM extensions) and what isn't (anything needing a React overlay). No behavioural change — the prop continues to be accepted. Removal is a separate decision for a future major version. Verified - npm test: 38/38 (tests reference state.history.canRedo which is unchanged) - npm run typecheck: clean - npm run lint: exit 0 Refs: #5, #26 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 23, 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
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.
Summary
Two API hygiene changes bundled into one PR — both small, both touching the public surface.
1. Replace
Partial<CanvasNode>with explicit operation variants (closes #5)The last FFI-hostile shape from the original audit.
Partial<T>has no clean Rust equivalent — it'd force a stringly-typedHashMap<String, JsonValue>or a sparse-field struct across JSI / wasm-bindgen. Replaced with six per-field operation variants:updateNodeColor(nodeId, color)updateTextNodeText(nodeId, text)updateLinkNodeUrl(nodeId, url)updateFileNode(nodeId, file, subpath)updateGroupNodeLabel(nodeId, label)updateGroupNodeBackground(nodeId, background, backgroundStyle)Each
Operationvariant carries the new value(s) and prev value(s) forinvertOperation. Apply paths verify node type before mutating (silent no-op on missing or mismatched type, consistent withremoveNode'sif (!node) returnpattern).Exported
GroupBackgroundStyle = 'cover' | 'ratio' | 'repeat'so consumers can type-narrow without reaching intoGroupNode.Breaking:
updateNodeContentis gone from the public API. No external consumers in this repo. Workspace's canvas-ui has the same shape and will need the same migration when it next pulls from upstream.2. Deprecate
renderMarkdown(closes #26)The prop on
CanvasViewwas vestigial — nothing insrc/renderer/**consumed it becauseTextNodeContentreturns null and all text-node bodies render throughparagraphBuilder/ Skia. Marked@deprecatedin both declaration sites (CanvasView.tsx,CanvasContext.tsx) with a JSDoc explaining what actually drives text rendering. README's "What's inside" section gains a Markdown rendering subsection enumerating supported features (inline marks, blocks, GFM extensions) and what isn't (anything needing a React overlay).No behavioural change — the prop continues to be accepted and ignored. Removal is a separate decision for a later major version.
Verification
npm test: 38/38 (tests referencestate.history.canRedowhich is unchanged)npm run typecheck: cleannpm run lint: exit 0Why bundle these
Both are public-API surface hygiene, both small, both serve the same forward direction (cleaner shape for a future Rust core, clearer contract for consumers). Splitting into two PRs is overhead for ~285 LOC across 6 files. If you'd rather they were split, easy to do.
What's next after this merges
Test plan
updateNodeContentwasn't called anyway)Refs: #5, #26