fix: bypass Notion image proxy for external bookmark URLs

[seonglae]

Summary

• External HTTPS URLs now bypass the notion.so/image/ proxy, fixing broken bookmark cover/icon images
• Added onError handling to bookmark images in LazyImage to hide broken images gracefully
• Added 7 tests for defaultMapImageUrl external URL handling

Test plan

• All 7 new map-image-url.test.ts tests pass
• Build succeeds across all packages
• Visual verification: bookmark images load correctly on texonom.com

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Lazy-loaded images now accept a custom error handler for better fallback control.
• Bug Fixes / Improvements
  • Bookmark icons and covers hide gracefully if image loading fails.
  • Collection views (table/board/list/card) are more robust when underlying schema is missing.
  • External HTTPS images are no longer proxied unnecessarily, improving reliability and performance.
• Tests
  • Added comprehensive tests for image URL mapping across various URL patterns.
• Chores
  • Updated a development tooling dependency and adjusted build configuration.

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds image load error handling and a LazyImage onError prop, tightens image URL proxying to skip external HTTPS hosts, bumps a dev dependency, updates Vite rollup externals, and adds tests for image URL mapping.

Changes

Cohort / File(s)	Summary
Build config & deps packages/cli/vite.config.ts, packages/nutils/package.json	Added 'child_process' to Rollup/Vite externals; bumped vite-plugin-dts devDependency from ^3.9.1 to ^4.5.4.
Image components packages/nreact/src/block.tsx, packages/nreact/src/components/lazy-image.tsx	Added onError handling: LazyImage accepts and forwards an optional onError prop; bookmark images in block.tsx hide their elements when image load fails.
Collection rendering guards packages/nreact/src/third-party/... (collection-card.tsx, collection-utils.ts, collection-view-board.tsx, collection-view-list.tsx, collection-view-table.tsx)	Replaced direct collection.schema[...] access with optional chaining (collection.schema?.[...]) and guarded Object.keys usage to tolerate missing/undefined schema.
Image URL mapping & tests packages/nutils/src/map-image-url.ts, packages/nutils/src/map-image-url.test.ts	Added bypass: for HTTPS URLs whose hostname does not end with notion.so or amazonaws.com, return original URL (skip proxy). Added comprehensive tests covering data URLs, Unsplash, notion-static, S3, relative paths, and external HTTPS URLs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Switch nutils to vite build #226: Modifies the same Vite/Rollup externals (adds child_process) in packages/cli/vite.config.ts.

Suggested labels

codex

Poem

Hop hop, a rabbit tweaks each img and URL,
When loads go awry, the broken bits curl.
External links pass by, proxies step aside,
Tests nibble at cases with confident stride.
Code hops along — a joyful developer ride! 🐇

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description lacks required sections from the repository template: no Notion Test Page ID is provided, and the description does not follow the specified template structure.	Add a 'Notion Test Page ID' section with a publicly accessible Notion page ID for testing and debugging, or note if one is not available.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly summarizes the main change: bypassing the Notion image proxy for external bookmark URLs, which directly relates to the primary fix in the changeset.

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

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/bookmark-rendering

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/nreact/src/third-party/collection-view-table.tsx`:
- Around line 70-74: The loop that renders table body cells reads schema =
collection.schema?.[property] and later dereferences schema.type causing a
TypeError when collection.schema is undefined; add a guard like in
collection-view-list.tsx so any access to schema.type first ensures schema is
truthy (e.g., use const schema = collection.schema?.[property] ?? null and skip
or use default when schema is null) inside the body-rendering loop (the code
that iterates over properties and reads schema.type), and ensure the
table_properties/properties path handles missing collection.schema by treating
missing schema entries as null/empty before accessing .type.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Incomplete fix — collection.schema || {} still leads to NPE at line 126 via the else branch.

When collection.schema is undefined and collectionView.format?.table_properties is falsy (the else branch), the fix changes Object.keys(collection.schema) (which threw immediately) into Object.keys({}), yielding []. The concat at line 70 still produces properties = [{ property: 'title' }]. The body-rendering loop then reaches line 112 where schema = collection.schema?.title → undefined, and line 126 dereferences schema.type → TypeError.

The crash is relocated from Object.keys() to schema.type, not eliminated.

Add a guard consistent with the pattern already used in collection-view-list.tsx (line 54):

🛡️ Proposed fix — guard before `schema.type` dereference in the body loop

  {properties.map(p => {
    const schema = collection.schema?.[p.property]
    const block = recordMap.block[blockId]?.value
    const data = block?.properties?.[p.property]
    const isTitle = p.property === 'title'
    const style: React.CSSProperties = {}

    if (p.width) style.width = p.width
    else if (isTitle) style.width = 280
    else style.width = 200
+
+   if (!schema) return null

    return (
      <div
        key={p.property}
        className={`notion-table-cell ${`notion-table-cell-${schema.type}`}`}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/nreact/src/third-party/collection-view-table.tsx` around lines 70 -
74, The loop that renders table body cells reads schema =
collection.schema?.[property] and later dereferences schema.type causing a
TypeError when collection.schema is undefined; add a guard like in
collection-view-list.tsx so any access to schema.type first ensures schema is
truthy (e.g., use const schema = collection.schema?.[property] ?? null and skip
or use default when schema is null) inside the body-rendering loop (the code
that iterates over properties and reads schema.type), and ensure the
table_properties/properties path handles missing collection.schema by treating
missing schema entries as null/empty before accessing .type.