You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@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
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.
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.
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.
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.
Context
@cluesmith/codev-artifact-canvas(spec 945, PR #1027) bundles two layers with different environment requirements:FileAdapter,MarkerAdapter,ThemeAdapter), data types (ReviewMarker,Disposable), andArtifactCanvasProps. Compile-time-only types, run anywhere.renderMarkdown/MarkdownView/ArtifactCanvas.markdown-itis universal, but the layer also usesDOMPurify.sanitize(needs a livewindow/document) and renders viadangerouslySetInnerHTML(areact-domDOM prop).react-domis 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
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.Do not pre-split now. Reasons:
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 thereact-dompeer.…-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.