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 support a custom error handler for improved fallback control.
• Bug Fixes
  • Bookmark icons and covers hide gracefully if image loading fails.
  • External HTTPS images are no longer proxied unnecessarily, reducing failures and improving performance.
• Tests
  • Added comprehensive tests for image URL mapping across multiple URL patterns.
• Chores
  • Updated a development dependency for tooling reliability.
  • Adjusted build configuration to improve bundling behavior.

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces image error handling across components, adds URL validation logic to prevent unnecessary proxy usage, extends the LazyImage component API, updates a build dependency, and adds comprehensive test coverage for URL mapping behavior.

Changes

Cohort / File(s)	Summary
Build Configuration & Dependencies packages/cli/vite.config.ts, packages/nutils/package.json	Added 'child_process' to Vite's external rollup dependencies; bumped vite-plugin-dts from ^3.9.1 to ^4.5.4.
Image Error Handling packages/nreact/src/block.tsx, packages/nreact/src/components/lazy-image.tsx	Added error handlers to bookmark images in block.tsx; extended LazyImage component with optional onError prop forwarded to underlying img element.
URL Mapping Logic & Tests packages/nutils/src/map-image-url.ts, packages/nutils/src/map-image-url.test.ts	Added bypass condition for HTTPS URLs not ending with notion.so or amazonaws.com to avoid unnecessary proxy routing; added 63-line test suite validating URL patterns across data URLs, Unsplash, S3, relative paths, and external HTTPS URLs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Switch nutils to vite build #226: Directly modifies the same vite.config.ts file by adding child_process to rollup externals, indicating related build configuration work.

Suggested labels

codex

Poem

Hop hop, the images now gracefully hide,
When errors appear, they smoothly subside!
URLs bypass when they should roam free,
Tests bloom like clover, one-two-three! 🐰
With props and with logic, we code with such glee!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description provides a clear summary of changes and test status, but does not follow the required template structure with 'Description' and 'Notion Test Page ID' sections.	Restructure the description to follow the template with separate 'Description' and 'Notion Test Page ID' sections. Include a Notion test page ID for debugging purposes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: bypassing the Notion image proxy for external bookmark URLs, which is the primary fix addressing broken bookmark images.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

packages/nreact/src/block.tsx (1)

543-550: Minor inconsistency: icon onError hides the <img> but leaves the parent .notion-bookmark-link-icon div visible.

For the bookmark cover (Line 569), you hide the parent wrapper via .closest(). Here, you only hide the <img> itself, which may leave an empty .notion-bookmark-link-icon <div> contributing unwanted spacing in the bookmark link row.

Suggested fix for consistency

                     onError={e => {
-                       const target = e.currentTarget as HTMLImageElement
-                       target.style.display = 'none'
+                       const parent = (e.currentTarget as HTMLImageElement).closest('.notion-bookmark-link-icon')
+                       if (parent instanceof HTMLElement) parent.style.display = 'none'
                     }}

packages/nutils/src/map-image-url.test.ts (2)

34-38: The assertion for proxied notion-static URLs could be more specific.

toContain('notion.so') is a weak assertion — even if the proxy logic broke and returned the input URL unchanged, the test would still pass since the input URL (https://www.notion.so/image/test.jpg) already contains notion.so. Consider asserting on the query parameters (table=block, id=test-block-id, cache=v2) that the proxy logic appends.

Suggested improvement

 test('proxies notion-static URLs through notion.so', () => {
   const url = 'https://www.notion.so/image/test.jpg'
   const result = defaultMapImageUrl(url, mockBlock)
-  expect(result).toContain('notion.so')
+  expect(result).toContain('notion.so')
+  expect(result).toContain('table=block')
+  expect(result).toContain('id=test-block-id')
+  expect(result).toContain('cache=v2')
 })

---

54-63: Consider adding a test for signed S3 URLs (with X-Amz-* params).

The existing test at Line 59 covers unsigned S3 URLs. Adding a case with X-Amz-Credential, X-Amz-Signature, and X-Amz-Algorithm query params would confirm the signed-URL early-return path works correctly.

packages/nreact/src/components/lazy-image.tsx (1)

18-19: onError is only forwarded to the native <img> — not to components.Image.

When previewImages or forceCustomImages triggers the components.Image path (Lines 47 and 77), the onError handler is silently dropped. This is likely fine for the current bookmark use case but could be surprising for future callers.

Worth a brief code comment or, if components.Image supports onError, forwarding it there too.

Also applies to: 99-99

---

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.}