Skip to content

M10 · React renderer package - #337

Merged
ViewFromTheBox merged 2 commits into
release/1.0from
feature/320-m10-react-renderer-package
Apr 20, 2026
Merged

ViewFromTheBox merged 2 commits into
release/1.0from
feature/320-m10-react-renderer-package

Conversation

@ViewFromTheBox

@ViewFromTheBox ViewFromTheBox commented Apr 20, 2026 •

Copy link
Copy Markdown
Member

Description

Ships the @artisanpack-ui/visual-editor-renderer-react npm package: a React
component that takes a block tree the visual editor persists and renders it
to React elements. Designed for Inertia+React apps. Markup mirrors the M9
Blade renderer one-to-one — same classes, attribute allow-lists, and URL
sanitization — so server- and client-rendered pages stay interchangeable.

Closes: #320

Type of Change

  • New feature (adds new functionality)

Related Issue

Issue: #320

Motivation and Context

Required for users running Inertia+React. Lives in its own npm package so
apps that don't need React on the frontend don't ship it. Shares the
registry + attribute-adapter pattern with the upcoming Vue renderer (M11).

Changes Made

  • New sub-package at packages/visual-editor-renderer-react/, distributed
    to npm via the same subtree-split workflow as the Blade renderer.
  • <BlockTree tree={...} /> walks the persisted tree and mounts each block
    through an overridable React-component registry.
  • 29 core block renderers across text / media / layout / design families —
    exactly the V1 allow-list from the M5 audit.
  • registerBlockRenderer(name, component) lets hosts add custom blocks or
    override core ones. unregister, reset, has, get round out the API.
  • <DynamicBlock> fetches /visual-editor/api/blocks/preview on mount for
    any block without a client-side renderer, falls back to the unknown-block
    marker on error. Endpoint + fetch options are prop-configurable.
  • URL sanitizer + attribute coercion helpers ported from the PHP side —
    javascript:, data:, vbscript: dropped on every URL-shaped attribute
    (image src/href, button url, video src/poster, audio src,
    file href/textLinkHref, media-text mediaUrl, cover url).
  • vitest.config.ts picks up package tests alongside the legacy suite.
  • PACKAGING.md registers the new sub-package in the subtree-split table.

How Has This Been Tested?

Testing Environment:

  • Operating System: macOS
  • Browser: Safari 26.3.1
  • PHP Version: 8.4.3
  • Project Version: 1.0.0-spike

Tests Performed:

  1. npm test — 350 vitest tests passing, 1 skipped.
  2. ./vendor/bin/pest — 136 PHP tests passing (unchanged).
  3. Manually verified the demo route at
    /packages/visual-editor/m10/post/{id} in the dev app: Inertia+React
    page renders the saved post's 4 paragraphs + 1 image through <BlockTree>.

Tests Added

  • Unit tests added/updated
  • All tests passing

Test details:

54 new vitest tests under packages/visual-editor-renderer-react/tests/:

  • BlockTree.test.tsx — every core block rendered from fixtures, normalization
    of null/undefined/JSON-string/array trees, custom renderer override, dynamic
    fallback, URL sanitization on image / button / cover / media-text, allow-lists
    for cover dim ratio / table cell align.
  • DynamicBlock.test.tsx — fetch HTML from the preview endpoint, 404 and
    rejected fetch both fall back to the unknown-block marker.
  • registry.test.ts — registration, override, unregister, reset, empty-name
    rejection, core block coverage.
  • urlSanitizer.test.ts — safe schemes pass through, unsafe schemes get dropped.

Documentation

  • Inline code documentation added
  • README updated

Documentation details:

  • Every module gets a short purpose docblock.
  • packages/visual-editor-renderer-react/README.md covers install, props,
    custom renderer API, dynamic-block fetch, and the core-block allow-list.
  • PACKAGING.md sub-package table updated with the new npm package.

Pre-Submission Checklist

  • Code passes all tests
  • Self-review completed
  • Comments added for complex code
  • No new warnings generated

Summary by CodeRabbit

  • New Features

    • New React package @artisanpack-ui/visual-editor-renderer-react: renders saved block trees, registers core block renderers, supports dynamic server-rendered previews, and exposes a registry API for custom renderers.
  • Documentation

    • Comprehensive README added with installation, API, props, and usage guidance.
  • Tests

    • Extensive test suites covering rendering, dynamic previews, registry behavior, and URL sanitization.
  • Chores

    • Packaging metadata and test discovery config updated.

Ships the `@artisanpack-ui/visual-editor-renderer-react` npm package for
Inertia+React hosts: a `<BlockTree>` component that walks the block tree
the editor persists, renders every core block from an overridable registry,
and falls back to the visual-editor preview endpoint for dynamic blocks.

Markup mirrors the M9 Blade renderer one-to-one (classes, attribute
allow-lists, URL sanitization) so server- and client-rendered pages stay
visually identical.

- packages/visual-editor-renderer-react: new npm sub-package
- 29 core block renderers: text / media / layout / design families
- `registerBlockRenderer(name, component)` for custom + override blocks
- `<DynamicBlock>` POSTs to `/visual-editor/api/blocks/preview` on mount
- URL sanitizer + attribute coercion helpers ported from the PHP side
- 54 vitest tests covering every block, registry, dynamic fetch, sanitizer
- vitest.config.ts: include `packages/**/tests` alongside the legacy suite
- PACKAGING.md: register the new sub-package in the subtree-split table

Closes #320
@coderabbitai

coderabbitai Bot commented Apr 20, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds a new npm package @artisanpack-ui/visual-editor-renderer-react that provides a BlockTree React renderer, a block renderer registry, core block renderer implementations, dynamic server-preview fetching, sanitization/utilities, and comprehensive tests and config updates.

Changes

Cohort / File(s) Summary
Packaging & package metadata
PACKAGING.md, packages/visual-editor-renderer-react/package.json, packages/visual-editor-renderer-react/README.md, packages/visual-editor-renderer-react/tsconfig.json
Added new scoped package metadata, README, and TypeScript config; updated PACKAGING.md to list the new React renderer package and adjust planned sub-packages.
Public entry & bootstrap
packages/visual-editor-renderer-react/src/index.ts
New package entry that auto-registers core blocks on import and re-exports public API (BlockTree, DynamicBlock, UnknownBlock, registry APIs, types).
Core rendering components
packages/visual-editor-renderer-react/src/BlockTree.tsx, packages/visual-editor-renderer-react/src/DynamicBlock.tsx, packages/visual-editor-renderer-react/src/blocks/unknownBlock.tsx
Implemented BlockTree recursive renderer, DynamicBlock (POST preview fetch + HTML injection / UnknownBlock fallback), and UnknownBlock fallback component.
Registry & types
packages/visual-editor-renderer-react/src/registry.ts, packages/visual-editor-renderer-react/src/types.ts
Added in-memory registry with register/unregister/lookup helpers and exported shared Block and BlockRenderer types.
Core block implementations
packages/visual-editor-renderer-react/src/blocks/.../text.tsx, .../layout.tsx, .../media.tsx, .../design.tsx, .../registerCoreBlocks.ts
Implemented numerous core block renderers across text, layout, media, and design families; added registerCoreBlocks() to populate the registry.
Support utilities
packages/visual-editor-renderer-react/src/support/attributes.ts, packages/visual-editor-renderer-react/src/support/urlSanitizer.ts
Added attribute coercion helpers (attrString/attrInt/attrBoolean/attrRecord/attrArray), classList, formatPercent, and safeUrl scheme allowlist sanitizer.
Tests & test config
packages/visual-editor-renderer-react/tests/*, vitest.config.ts
Added comprehensive Vitest suites for BlockTree, DynamicBlock, registry, url sanitizer, and helpers; extended vitest config to include packages/**/tests/ discovery.

Sequence Diagram

sequenceDiagram
    actor User
    participant BlockTree
    participant Registry
    participant CoreRenderer
    participant DynamicBlock
    participant PreviewAPI

    User->>BlockTree: render(tree)
    activate BlockTree

    loop each block
        BlockTree->>Registry: getBlockRenderer(name)
        Registry-->>BlockTree: renderer | undefined

        alt renderer exists
            BlockTree->>CoreRenderer: render(props)
            CoreRenderer-->>BlockTree: JSX
            opt innerBlocks
                BlockTree->>BlockTree: recursively render children
            end
        else no renderer
            BlockTree->>DynamicBlock: mount(name, attributes, endpoint)
            DynamicBlock->>PreviewAPI: POST /visual-editor/api/blocks/preview {name, attributes}
            PreviewAPI-->>DynamicBlock: { html } or error
            alt success
                DynamicBlock-->>BlockTree: preview HTML (dangerouslySetInnerHTML)
            else failure
                DynamicBlock-->>BlockTree: UnknownBlock marker
            end
        end
    end

    BlockTree-->>User: rendered output
    deactivate BlockTree
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hop through trees of blocks so neat,
Registering renderers, tidy and fleet,
Fetching previews when client code's thin,
Sewing HTML snippets back in,
A tiny rabbit cheers: "Render and win!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'M10 · React renderer package' is specific and directly describes the main change: introducing a new React renderer package, matching the milestone and PR objectives.
Linked Issues check ✅ Passed All acceptance criteria from issue #320 are satisfied: new package created, BlockTree component implemented, core-block registry with 29 renderers, dynamic blocks with fetch fallback, registerBlockRenderer API, demo page verification, and comprehensive tests covering all core block types.
Out of Scope Changes check ✅ Passed Changes are within scope: new React renderer package (packages/visual-editor-renderer-react with 29 core block renderers, registry, BlockTree/DynamicBlock components), supporting helpers, tests, and minimal updates to PACKAGING.md and vitest.config.ts. All align with issue #320 objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/320-m10-react-renderer-package

Comment @coderabbitai help to get the list of available commands and usage tips.

@ViewFromTheBox ViewFromTheBox moved this from Backlog to In Review in ArtisanPack UI Overview Apr 20, 2026
@ViewFromTheBox ViewFromTheBox linked an issue Apr 20, 2026 that may be closed by this pull request
7 tasks
@ViewFromTheBox
ViewFromTheBox marked this pull request as ready for review April 20, 2026 15:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (3)
packages/visual-editor-renderer-react/tests/registry.test.ts (1)

22-29: Consider testing the inverse trim scenario.

The test verifies registration with whitespace and lookup without. For completeness, consider also testing lookup with whitespace when registered without:

registerBlockRenderer('acme/clean', renderer);
expect(getBlockRenderer('  acme/clean  ')).toBe(renderer);

This confirms bidirectional trim behavior in getBlockRenderer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor-renderer-react/tests/registry.test.ts` around lines 22
- 29, Add a complementary test to verify bidirectional trimming: register a
renderer with a clean name using registerBlockRenderer('acme/clean', renderer)
and assert that getBlockRenderer('  acme/clean  ') returns that renderer and
hasBlockRenderer('  acme/clean  ') is true; this ensures getBlockRenderer and
hasBlockRenderer trim input symmetrically in addition to the existing test that
trims on registration.
packages/visual-editor-renderer-react/src/DynamicBlock.tsx (1)

97-97: fetchOptions object in dependency array may cause unnecessary re-fetches.

If the parent component doesn't memoize fetchOptions, every render creates a new object reference, triggering the effect and re-fetching the preview. Consider either:

  1. Documenting that consumers should memoize fetchOptions
  2. Using a stable serialization for comparison (similar to attributesJson)
♻️ Option: serialize fetchOptions for stable deps
+ const fetchOptionsJson = useMemo(
+     () => JSON.stringify(fetchOptions ?? {}),
+     [fetchOptions]
+ );
+
  useEffect(() => {
      // ...
- }, [name, endpoint, fetchFn, fetchOptions, attributesJson]);
+ }, [name, endpoint, fetchFn, fetchOptionsJson, attributesJson]);

Then reconstruct the options inside the effect from JSON.parse(fetchOptionsJson). Note: this approach doesn't handle non-serializable options like custom signal objects.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor-renderer-react/src/DynamicBlock.tsx` at line 97, The
effect in DynamicBlock.tsx is re-running unnecessarily because the plain object
fetchOptions is unstable; replace it in the dependency array with a stable
serialized value (e.g. compute fetchOptionsJson = useMemo(() =>
JSON.stringify(fetchOptions), [fetchOptions]) and use fetchOptionsJson in the
deps) and inside the effect reconstruct the options by parsing that JSON before
using fetchFn; keep consumers' non-serializable options (like AbortSignal) in
mind and document that those should be memoized or passed separately if needed.
packages/visual-editor-renderer-react/package.json (1)

8-15: Document the bundler requirement in the package.

The package intentionally exports raw TypeScript source (./src/index.ts) because it's designed for Inertia+React apps, which always use a bundler (Vite, webpack, etc.). However, the package.json doesn't explicitly state this requirement. Add a note to the README or package.json clarifying that consumers must use a bundler that handles TypeScript, or update the package.json description to make this clearer (e.g., "Requires a bundler with TypeScript support").

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor-renderer-react/package.json` around lines 8 - 15,
Update the package metadata to document that the package exports raw TypeScript
source and therefore requires a bundler that understands TypeScript: modify the
package description or add a short note in package.json (e.g., in "description")
stating "Requires a bundler with TypeScript support (Vite, webpack, etc.)"
and/or add a README blurb calling out that the package intentionally exports
"./src/index.ts" via "main", "types", and "exports" and must be consumed by
Inertia+React apps using a bundler that transpiles TypeScript.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/visual-editor-renderer-react/src/blocks/core/design.tsx`:
- Around line 236-239: The generated inputId (variable inputId created via
useMemo using stableHash over name and attributes) is identical for identical
blocks causing duplicate DOM ids; replace this by using React's useId() to
produce a per-instance stable identifier (optionally concatenated with the
existing stableHash or name to preserve readability) and use that id for the
input and label htmlFor instead of the current useMemo/stableHash approach so
each core/search block instance gets a unique id.

In `@packages/visual-editor-renderer-react/src/blocks/core/text.tsx`:
- Line 23: The content passed into dangerouslySetInnerHTML (produced via
attrString()/content in text.tsx and the similar variables in media.tsx,
layout.tsx, design.tsx and other core block renderers) is not sanitized; replace
the direct use with a sanitized string by running the attribute/HTML value
through a trusted sanitizer (e.g., DOMPurify or sanitize-html) before assigning
to __html, and do the same wherever attrString() output is injected (refer to
attrString and the local variable named content in text.tsx) so all HTML-bearing
attributes are cleaned of scripts/unsafe tags/attributes prior to render.

In `@packages/visual-editor-renderer-react/src/BlockTree.tsx`:
- Around line 62-70: innerBlocks may contain non-block values and is not being
normalized before recursion, causing renderBlock to receive invalid nodes;
before mapping or passing innerBlocks to renderers (the innerBlocks variable
used when computing renderedChildren and anywhere passed into renderBlock/custom
renderers), filter it with isBlock() (e.g., const innerBlocksFiltered =
Array.isArray(block.innerBlocks) ? block.innerBlocks.filter(isBlock) : []) and
use that filtered array for the ternary map and when calling renderBlock so only
valid block objects are recursed into and forwarded to custom renderers.

In `@packages/visual-editor-renderer-react/src/support/attributes.ts`:
- Around line 30-32: The string branch (if (typeof value === 'string'))
currently compares the raw value and fails for trimmed or case variants;
normalize the input before testing by trimming and lowercasing it (e.g. let
normalized = value.trim().toLowerCase()) and then evaluate normalized !== '' &&
normalized !== 'false' && normalized !== '0' so that values like ' FALSE ', '0
', or ' False' are correctly treated as falsey; update any return in that branch
to use the normalized variable.

In `@packages/visual-editor-renderer-react/tests/urlSanitizer.test.ts`:
- Around line 23-28: The test in urlSanitizer.test.ts claims to allow ftp and
sms schemes but doesn't assert them; update the it('allows http, https, mailto,
tel, ftp, and sms schemes', ...) block to add assertions calling
safeUrl('ftp://example.test') and safeUrl('sms:+15555551212') and expect them to
return the same strings, alongside the existing safeUrl checks—this ensures the
safeUrl function is actually verified for ftp and sms schemes.

---

Nitpick comments:
In `@packages/visual-editor-renderer-react/package.json`:
- Around line 8-15: Update the package metadata to document that the package
exports raw TypeScript source and therefore requires a bundler that understands
TypeScript: modify the package description or add a short note in package.json
(e.g., in "description") stating "Requires a bundler with TypeScript support
(Vite, webpack, etc.)" and/or add a README blurb calling out that the package
intentionally exports "./src/index.ts" via "main", "types", and "exports" and
must be consumed by Inertia+React apps using a bundler that transpiles
TypeScript.

In `@packages/visual-editor-renderer-react/src/DynamicBlock.tsx`:
- Line 97: The effect in DynamicBlock.tsx is re-running unnecessarily because
the plain object fetchOptions is unstable; replace it in the dependency array
with a stable serialized value (e.g. compute fetchOptionsJson = useMemo(() =>
JSON.stringify(fetchOptions), [fetchOptions]) and use fetchOptionsJson in the
deps) and inside the effect reconstruct the options by parsing that JSON before
using fetchFn; keep consumers' non-serializable options (like AbortSignal) in
mind and document that those should be memoized or passed separately if needed.

In `@packages/visual-editor-renderer-react/tests/registry.test.ts`:
- Around line 22-29: Add a complementary test to verify bidirectional trimming:
register a renderer with a clean name using registerBlockRenderer('acme/clean',
renderer) and assert that getBlockRenderer('  acme/clean  ') returns that
renderer and hasBlockRenderer('  acme/clean  ') is true; this ensures
getBlockRenderer and hasBlockRenderer trim input symmetrically in addition to
the existing test that trims on registration.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d8213060-8d6c-49cf-a1a8-aa1febea276f

📥 Commits

Reviewing files that changed from the base of the PR and between 1fd4985 and 8c7a3ce.

📒 Files selected for processing (23)
  • PACKAGING.md
  • packages/visual-editor-renderer-react/README.md
  • packages/visual-editor-renderer-react/package.json
  • packages/visual-editor-renderer-react/src/BlockTree.tsx
  • packages/visual-editor-renderer-react/src/DynamicBlock.tsx
  • packages/visual-editor-renderer-react/src/blocks/core/design.tsx
  • packages/visual-editor-renderer-react/src/blocks/core/layout.tsx
  • packages/visual-editor-renderer-react/src/blocks/core/media.tsx
  • packages/visual-editor-renderer-react/src/blocks/core/text.tsx
  • packages/visual-editor-renderer-react/src/blocks/registerCoreBlocks.ts
  • packages/visual-editor-renderer-react/src/blocks/unknownBlock.tsx
  • packages/visual-editor-renderer-react/src/index.ts
  • packages/visual-editor-renderer-react/src/registry.ts
  • packages/visual-editor-renderer-react/src/support/attributes.ts
  • packages/visual-editor-renderer-react/src/support/urlSanitizer.ts
  • packages/visual-editor-renderer-react/src/types.ts
  • packages/visual-editor-renderer-react/tests/BlockTree.test.tsx
  • packages/visual-editor-renderer-react/tests/DynamicBlock.test.tsx
  • packages/visual-editor-renderer-react/tests/helpers.ts
  • packages/visual-editor-renderer-react/tests/registry.test.ts
  • packages/visual-editor-renderer-react/tests/urlSanitizer.test.ts
  • packages/visual-editor-renderer-react/tsconfig.json
  • vitest.config.ts

Comment thread packages/visual-editor-renderer-react/src/blocks/core/design.tsx Outdated
className,
]);

return <p className={classes} dangerouslySetInnerHTML={{ __html: content }} />;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "text.tsx" | grep visual-editor-renderer-react

Repository: ArtisanPack-UI/visual-editor

Length of output: 136


🏁 Script executed:

cat -n packages/visual-editor-renderer-react/src/blocks/core/text.tsx

Repository: ArtisanPack-UI/visual-editor

Length of output: 6467


🏁 Script executed:

cd packages/visual-editor-renderer-react && find . -type f -name "*.ts" -o -name "*.tsx" | head -20

Repository: ArtisanPack-UI/visual-editor

Length of output: 528


🏁 Script executed:

grep -r "function attrString\|const attrString\|export.*attrString" packages/visual-editor-renderer-react/

Repository: ArtisanPack-UI/visual-editor

Length of output: 203


🏁 Script executed:

cat -n packages/visual-editor-renderer-react/src/support/attributes.ts

Repository: ArtisanPack-UI/visual-editor

Length of output: 3332


🏁 Script executed:

cat -n packages/visual-editor-renderer-react/src/support/urlSanitizer.ts

Repository: ArtisanPack-UI/visual-editor

Length of output: 1238


🏁 Script executed:

grep -n "dangerouslySetInnerHTML" packages/visual-editor-renderer-react/src/blocks/core/media.tsx packages/visual-editor-renderer-react/src/blocks/core/layout.tsx packages/visual-editor-renderer-react/src/blocks/core/design.tsx

Repository: ArtisanPack-UI/visual-editor

Length of output: 1499


🏁 Script executed:

grep -r "sanitize\|DOMPurify\|xss\|innerHTML" packages/visual-editor-renderer-react/src --include="*.ts" --include="*.tsx" | grep -v "dangerouslySetInnerHTML" | head -20

Repository: ArtisanPack-UI/visual-editor

Length of output: 632


🏁 Script executed:

grep -r "import.*sanitize\|from.*sanitize\|require.*sanitize" packages/visual-editor-renderer-react/ | head -10

Repository: ArtisanPack-UI/visual-editor

Length of output: 54


🏁 Script executed:

cat packages/visual-editor-renderer-react/package.json | grep -A 20 -B 5 "dependencies\|devDependencies" | grep -E "sanitize|purify|xss|html"

Repository: ArtisanPack-UI/visual-editor

Length of output: 54


🏁 Script executed:

head -50 packages/visual-editor-renderer-react/src/BlockTree.tsx

Repository: ArtisanPack-UI/visual-editor

Length of output: 1623


🏁 Script executed:

grep -r "sanitize\|trust\|safe" packages/visual-editor-renderer-react/src --include="*.ts" --include="*.tsx" -B 2 -A 2 | head -40

Repository: ArtisanPack-UI/visual-editor

Length of output: 3833


🏁 Script executed:

cat packages/visual-editor-renderer-react/README.md 2>/dev/null || cat packages/visual-editor-renderer-react/package.json | head -30

Repository: ArtisanPack-UI/visual-editor

Length of output: 4104


Sanitize HTML-bearing attributes before rendering.

attrString() only coerces types; it does not sanitize HTML. Block attributes passed to dangerouslySetInnerHTML at these lines (and in media.tsx, layout.tsx, design.tsx) can execute arbitrary scripts if the persisted tree includes crafted HTML content. The same pattern appears across multiple core block renderers.

Also applies to: 41, 72–74, 81–86, 101, 112, 121, 133, 146–149

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor-renderer-react/src/blocks/core/text.tsx` at line 23,
The content passed into dangerouslySetInnerHTML (produced via
attrString()/content in text.tsx and the similar variables in media.tsx,
layout.tsx, design.tsx and other core block renderers) is not sanitized; replace
the direct use with a sanitized string by running the attribute/HTML value
through a trusted sanitizer (e.g., DOMPurify or sanitize-html) before assigning
to __html, and do the same wherever attrString() output is injected (refer to
attrString and the local variable named content in text.tsx) so all HTML-bearing
attributes are cleaned of scripts/unsafe tags/attributes prior to render.

Comment thread packages/visual-editor-renderer-react/src/BlockTree.tsx Outdated
Comment thread packages/visual-editor-renderer-react/src/support/attributes.ts
Comment thread packages/visual-editor-renderer-react/tests/urlSanitizer.test.ts
Post-open review turned up four spots that needed tightening:

- SearchBlock now uses React's useId() for the input id instead of hashing
  attributes — two search blocks with identical attributes now get distinct
  DOM ids, which matters for forms with duplicate search widgets on one page.
- BlockTree.renderBlock filters innerBlocks through isBlock() before
  recursing, so custom renderers receiving the innerBlocks prop see only
  well-shaped block objects (the render loop already guarded on name, but
  the forwarded prop did not).
- attrBoolean normalizes string input (trim + lowercase) before comparing
  against 'false'/'0', so values like ' FALSE ' or ' 0 ' correctly coerce
  to false.
- urlSanitizer.test.ts asserts the ftp:// and sms: schemes it claims to
  cover, closing a documentation/test gap.

Skipping one finding: CR flagged every `dangerouslySetInnerHTML` site as
unsanitized. The project's trust model is that Gutenberg validates on save
and both renderers pass stored HTML through; the Blade side already uses
`{!! $content !!}` unescaped for the same attributes. Sanitizing at render
time would diverge from M9 output, add a DOMPurify dependency, and risk
stripping legitimate inline markup (<em>, <strong>, <a>).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/visual-editor-renderer-react/src/BlockTree.tsx (1)

63-63: Trim clientId before using it as a React key.

On Line 63, a whitespace-only clientId is currently accepted as a key. Trimming first avoids accidental blank/unstable keys.

Proposed tweak
-    const key = typeof block.clientId === 'string' && block.clientId !== '' ? block.clientId : `${name}-${index}`;
+    const clientId = typeof block.clientId === 'string' ? block.clientId.trim() : '';
+    const key = clientId !== '' ? clientId : `${name}-${index}`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor-renderer-react/src/BlockTree.tsx` at line 63, The
React key currently accepts whitespace-only clientId; update the key computation
in BlockTree.tsx so you trim block.clientId before checking emptiness and before
using it as the key. Specifically, in the expression that sets the variable key,
replace the direct check of block.clientId with a trimmed value (e.g., compute a
trimmedId from block.clientId, ensure trimmedId is non-empty) and use trimmedId
as the key; fall back to the existing `${name}-${index}` when trimmedId is empty
or not a string. Ensure references to block.clientId in this assignment use the
trimmed value consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/visual-editor-renderer-react/src/blocks/core/design.tsx`:
- Around line 225-227: buttonUseIcon currently clears buttonLabel causing an
unlabeled submit button; update the logic around
attrBoolean(attributes.buttonUseIcon) and buttonLabel so when buttonUseIcon is
true you render icon markup and still provide an accessible name (e.g., set
buttonLabel to buttonText or queryName for aria-label) instead of ''.
Specifically, change the assignment of buttonLabel and the button render paths
that reference buttonLabel (the blocks using buttonUseIcon/buttonLabel) so the
visual icon is injected and the button receives an accessible label via
aria-label (fallback to buttonText or queryName if no explicit label) while
keeping the non-icon case unchanged.

---

Nitpick comments:
In `@packages/visual-editor-renderer-react/src/BlockTree.tsx`:
- Line 63: The React key currently accepts whitespace-only clientId; update the
key computation in BlockTree.tsx so you trim block.clientId before checking
emptiness and before using it as the key. Specifically, in the expression that
sets the variable key, replace the direct check of block.clientId with a trimmed
value (e.g., compute a trimmedId from block.clientId, ensure trimmedId is
non-empty) and use trimmedId as the key; fall back to the existing
`${name}-${index}` when trimmedId is empty or not a string. Ensure references to
block.clientId in this assignment use the trimmed value consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 939d4291-8057-4083-a2ba-d86e7f81de6a

📥 Commits

Reviewing files that changed from the base of the PR and between 8c7a3ce and 5db2fa4.

📒 Files selected for processing (5)
  • packages/visual-editor-renderer-react/src/BlockTree.tsx
  • packages/visual-editor-renderer-react/src/blocks/core/design.tsx
  • packages/visual-editor-renderer-react/src/support/attributes.ts
  • packages/visual-editor-renderer-react/tests/BlockTree.test.tsx
  • packages/visual-editor-renderer-react/tests/urlSanitizer.test.ts
✅ Files skipped from review due to trivial changes (2)
  • packages/visual-editor-renderer-react/tests/urlSanitizer.test.ts
  • packages/visual-editor-renderer-react/tests/BlockTree.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/visual-editor-renderer-react/src/support/attributes.ts

Comment on lines +225 to +227
const useIcon = attrBoolean(attributes.buttonUseIcon);
const queryName = attrString(attrRecord(attributes.query).name, 's');
const buttonLabel = useIcon ? '' : buttonText;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

buttonUseIcon currently renders an empty, unlabeled button.

When buttonUseIcon is true, buttonLabel is forced to '', but the button never gets replacement icon markup or an accessible name. That leaves the icon variant visually blank and exposes an unnamed submit button to assistive tech.

Also applies to: 253-255

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor-renderer-react/src/blocks/core/design.tsx` around
lines 225 - 227, buttonUseIcon currently clears buttonLabel causing an unlabeled
submit button; update the logic around attrBoolean(attributes.buttonUseIcon) and
buttonLabel so when buttonUseIcon is true you render icon markup and still
provide an accessible name (e.g., set buttonLabel to buttonText or queryName for
aria-label) instead of ''. Specifically, change the assignment of buttonLabel
and the button render paths that reference buttonLabel (the blocks using
buttonUseIcon/buttonLabel) so the visual icon is injected and the button
receives an accessible label via aria-label (fallback to buttonText or queryName
if no explicit label) while keeping the non-icon case unchanged.

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

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

M10 · React renderer package

1 participant