Skip to content

artifact-canvas: defer a core/web/native package split until native rendering is a committed requirement (decision record) #1029

Description

@amrmelsayed

Context

@cluesmith/codev-artifact-canvas (spec 945, PR #1027) bundles two layers with different environment requirements:

  • Universal contract (no DOM): the adapter interfaces (FileAdapter, MarkerAdapter, ThemeAdapter), data types (ReviewMarker, Disposable), and ArtifactCanvasProps. Compile-time-only types, run anywhere.
  • DOM-bound rendering layer:renderMarkdown / MarkdownView / ArtifactCanvas. markdown-it is universal, but the layer also uses DOMPurify.sanitize (needs a live window/document) and renders via dangerouslySetInnerHTML (a react-dom DOM prop). react-dom is a peer dependency.

Consequence (confirmed): a React Native project can import and implement the type/adapter contract, but it cannot use the runtime exports to render natively (no div/innerHTML, no DOM for DOMPurify). Mobile works today only via a WebView (a WebView is a DOM environment, so the full package runs there unchanged). VSCode-webview, the dashboard, and mobile-in-a-WebView all share the same DOM rendering model.

The question

Should the package split into …-core / …-web / …-native, or stay monolithic with each client implementing its own rendering?

Decision

  1. Rejected: each client implements its own rendering. That defeats the package's purpose. The DOM-model hosts (VSCode webview, dashboard, mobile WebView) share one rendering model, so pushing rendering down to each client just re-duplicates the DOMPurify + innerHTML + data-line logic the package exists to share. Shared rendering belongs to hosts that share a model, not to every client.

  2. Do not pre-split now. Reasons:

    • No consumers yet. Host integration is vscode: add review comments from the markdown preview pane (hover-+ per block, no editor mode-switch) #859 (future). Nothing depends on the package's shape, so a later restructure is nearly free and breaks no one. Pre-splitting is speculative generality.
    • The split is a renderer redesign, not a folder move. A genuinely reusable core cannot emit an HTML string (that is already a DOM-shaped decision baked together with DOMPurify). It would have to emit the markdown token AST plus the data-line mapping plus the comment-intent logic, and let each surface turn that into its own output. That is real design work and only pays off if native happens.
    • The current package is a coherent v1: a web/DOM artifact canvas with an environment-agnostic contract. The only genuine flaw is that the name oversells platform-agnosticism; that is a one-paragraph README "Environment" note, not an architecture defect.
  3. If/when native React Native rendering becomes a committed requirement, split along the rendering model:

    • …-core: types + adapter interfaces + markdown-to-AST + data-line mapping + the comment-intent state machine. Emits structured tokens, no DOM.
    • …-web: AST to DOMPurify-sanitized HTML React component (today's renderer, slimmed to consume core). Keeps the react-dom peer.
    • …-native: AST to React Native view tree (<View>/<Text>) plus a non-DOM sanitization story. Note the XSS threat model differs when rendering to native views instead of HTML, so it is not just "swap DOMPurify."

Trigger to revisit

A committed native-RN rendering target. Until then, mobile is served via a WebView and no split is warranted.

Follow-ups (small, separable)

Out of scope

Performing the split or renaming the package now.

Origin

Surfaced while reviewing PR #1027 (spec 945). Relates to #859 (host integration) and #1028 (render-time-attribute pattern). The current package is correct for every DOM host that exists today; this records the layering decision so #859's integrator and any future native effort do not rediscover the reasoning.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/cross-cuttingTouches multiple areas — needs coordinated handling

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions