Skip to content

refactor(core): explicit per-field operation variants + deprecate renderMarkdown#27

Open
LeslieOA wants to merge 1 commit into
developfrom
refactor/explicit-update-variants
Open

refactor(core): explicit per-field operation variants + deprecate renderMarkdown#27
LeslieOA wants to merge 1 commit into
developfrom
refactor/explicit-update-variants

Conversation

@LeslieOA

Copy link
Copy Markdown
Member

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-typed HashMap<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 Operation variant carries the new value(s) and prev value(s) for invertOperation. 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 without reaching into GroupNode.

Breaking: updateNodeContent is 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 CanvasView was vestigial — nothing in src/renderer/** consumed it because TextNodeContent returns null and all text-node bodies render through paragraphBuilder / Skia. Marked @deprecated in 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 reference state.history.canRedo which is unchanged)
  • npm run typecheck: clean
  • npm run lint: exit 0

Why 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

  • Library checks (test / typecheck / lint)
  • Smoke test in playground (no behavioural change expected — updateNodeContent wasn't called anyway)

Refs: #5, #26

…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant