Skip to content

ENG-1992 Downgrade Obsidian react version to 18#1227

Open
trangdoan982 wants to merge 4 commits into
mainfrom
eng-1992-downgrade-obsidian-react-version-to-18
Open

ENG-1992 Downgrade Obsidian react version to 18#1227
trangdoan982 wants to merge 4 commits into
mainfrom
eng-1992-downgrade-obsidian-react-version-to-18

Conversation

@trangdoan982

@trangdoan982 trangdoan982 commented Jul 12, 2026

Copy link
Copy Markdown
Member

What & why

React 19's react-dom bundles createElement("script") (from its hoistable-resource / preload/preinit APIs), which trips the Obsidian community-plugin store's automated review. Root cause was confirmed in ENG-1988. The community-recommended fix is to pin the plugin to React 18.

This downgrades the Obsidian app from React 19.0.018.2.0.

Changes

  • pnpm-workspace.yaml — set catalog:obsidian react/react-dom to 18.2.0 and @types/* to 18.2.21/18.2.17, matching catalog:roam so both apps dedupe to a single React copy. tldraw@3.14.2 peer-accepts ^18.2.0 || ^19, so no patch is needed.
  • pnpm-lock.yaml — regenerated by pnpm install.
  • TldrawViewComponent.tsx — widened two manually-assigned refs to useRef<T | null>(null); React 18 types make RefObject.current read-only otherwise.

No React-19-only APIs are used, and no shared @repo/* package on Obsidian's path depends on React (@repo/ui is website-only), so this is isolated to the Obsidian app.

Verification

  • ✅ Rebuilt apps/obsidian/dist/main.js contains 0 createElement("script") occurrences (was 3) and carries React 18.2.0.
  • turbo check-types --filter=@discourse-graphs/obsidian passes.
  • ✅ Obsidian lint passes (0 errors).
  • ✅ Manual runtime smoke test in Obsidian (settings, modals, Discourse Context view, tldraw canvas) — recommended before merge; no automated Obsidian test suite exists.

🤖 Generated with Claude Code


Open in Devin Review

React 19's react-dom bundles createElement("script") from its hoistable
resource/preload APIs, which trips the Obsidian community-plugin store's
automated review (flagged in ENG-1988). Pin the Obsidian app to React 18.2.0
to remove the pattern.

- Set catalog:obsidian react/react-dom to 18.2.0 (matching catalog:roam so
  both apps dedupe to a single React copy). tldraw@3.14.2 peer-accepts ^18.
- Widen two manually-assigned refs to useRef<T | null>(null); React 18 types
  make RefObject.current read-only otherwise.

Verified: rebuilt dist/main.js has 0 createElement("script") occurrences
(was 3) and carries React 18.2.0; check-types and Obsidian lint pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 12, 2026

Copy link
Copy Markdown

ENG-1992

@graphite-app

graphite-app Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

@vercel

vercel Bot commented Jul 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
discourse-graph Error Error Jul 13, 2026 9:41pm

Request Review

@supabase

supabase Bot commented Jul 12, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

Fixes packages/ui type-check break from the previous commit. The workspace
shares a single hoisted @types/react; changing obsidian's @types to 18 flipped
that global slot, leaking React 18 types into type-less deps (lucide-react,
sonner) consumed by React 19 packages/ui.

Only the runtime react/react-dom (which the bundle depends on) needs to be 18.2.0
to remove createElement("script"). @types stay on ^19 as before, so the hoist
slot is unchanged and packages/ui is untouched. Reverts the now-unneeded useRef
typing change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With obsidian on react 18.2.0 and website/ui on 19, pnpm hoisted a single
react into the shared .pnpm store, and the version it picked flipped to 18.
Vercel's pruned website-only build then dangled on .pnpm/node_modules/react
(18.2.0 not in its graph) -> ENOENT.

Exclude react/react-dom from hoisting so no shared bare copy exists; every
package resolves its own anchored react via its declared dep/peer. Verified:
full check-types 8/8, obsidian bundle has 0 createElement("script") and React
18.2.0, website builds clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Testing showed hoistPattern was unnecessary (the website builds cleanly with
react@18 hoisted) and it could not fix the Vercel failure anyway: it only
applies on a clean node_modules, and Vercel reuses a cached one. The Vercel
red is build-cache staleness from the react 19->18 transition, not a code
issue, and is resolved by a clean install rather than a workspace change.

Net change is now just catalog:obsidian react/react-dom -> 18.2.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread pnpm-workspace.yaml
Comment on lines 43 to 44
"@types/react": ^19.0.12
"@types/react-dom": ^19.0.4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Critical Type Mismatch: The PR description states that @types/* should be downgraded to 18.2.21/18.2.17 to match React 18.2.0, but these lines show the types are still pinned to React 19 versions (^19.0.12 and ^19.0.4). This creates a dangerous mismatch where:

  1. Runtime uses React 18.2.0
  2. TypeScript uses React 19 type definitions

This will cause runtime errors when code compiles successfully but uses React 19-only APIs that don't exist in React 18.

Fix:

obsidian:
  "@types/react": 18.2.21
  "@types/react-dom": 18.2.17
  react: 18.2.0
  react-dom: 18.2.0
Suggested change
"@types/react": ^19.0.12
"@types/react-dom": ^19.0.4
"@types/react": 18.2.21
"@types/react-dom": 18.2.17

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

see comment

@trangdoan982

trangdoan982 commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

Design note: why obsidian keeps @types/react@^19 while runtime is react@18

This PR only touches the runtime react/react-dom versions for obsidian (19 → 18.2.0). The @types/react stays at ^19. Here's are options I've tried that didn't work:

Option 1 — Downgrade lucide-react / radix-ui in packages/ui to React-18-only versions

website (which uses Next.js 15 async server components) fails because its compiled-inline imports from @repo/ui now carry ReactElement from @types@18, which is incompatible with the ReactNode constraint in @types@19's JSX namespace.


Option 2 — Pin @types/react to 18.x in packages/ui (and/or in the obsidian catalog)

same error as above.

There is no pnpm API that scopes the shared hoist slot to a subtree — overrides, packageExtensions, and hoistPattern all have side-effects that cascade elsewhere.


Option 3 — Add hoistPattern to exclude react from hoisting (or an explicit overrides for website)

hoistPattern is not recorded in pnpm-lock.yaml. Vercel (and any other CI that restores a cached node_modules) sees no lock change → skips relinking → the change never applies. We verified this by running clean-clone tests: fresh install passes with or without hoistPattern. The failure only reproduces when installing on top of a stale node_modules from the React 19 era.

We confirmed in a Vercel clean rebuild that the site builds fine without hoistPattern, so it was reverted as unnecessary complexity.


Why the types/runtime skew is safe

@types/react is compile-time only — it never appears in the bundle. The Obsidian plugin store's automated review flags createElement("script") which comes exclusively from React 19's runtime react-dom (preload/preinit hoistable-resource APIs). That pattern is gone from the bundle with react-dom@18.2.0 regardless of which @types/react version TypeScript uses. We verified: grep -c 'createElement("script")' apps/obsidian/dist/main.js0 (was 3).

The only real risk from the skew is that a future React-19-only runtime API would type-check (since @types@19 would accept it) but crash on the 18 runtime. That's a known, accepted trade-off — no React-19-only APIs are used in the obsidian plugin today.

@trangdoan982 trangdoan982 force-pushed the eng-1992-downgrade-obsidian-react-version-to-18 branch from 4150aa1 to d2b1a03 Compare July 13, 2026 21:35
@trangdoan982 trangdoan982 force-pushed the eng-1992-downgrade-obsidian-react-version-to-18 branch from 592789e to 7c13533 Compare July 13, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant