Skip to content

fix(solid): fix hoisting for solid #5654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix(solid): fix hoisting for solid #5654

wants to merge 2 commits into from

Conversation

@birkskyum
Copy link
Member

@birkskyum birkskyum commented Oct 27, 2025 •
edited by coderabbitai bot
Loading

closes #5646

export function declarations in ES modules are not hoisted - they're treated like export const foo = function(),
creating a temporal dead zone. When you write:

  export const Route = createFileRoute('/posts/$postId')({
    errorComponent: PostErrorComponent,  // ❌ ReferenceError!
  })

  export function PostErrorComponent() { ... }

This causes: ReferenceError: Cannot access 'PostErrorComponent' before initialization

For solid, this PR wraps exported component references in getter functions which defers evaluation until after the component is declared.

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes

    • Fixed component reference handling to avoid runtime initialization issues in Solid-targeted builds.
    • Suppressed certain inline build warnings for Solid targets to reduce noisy outputs.

  • Refactor

    • Reordered an error component declaration for clearer code structure without changing behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025 •
edited
Loading

Walkthrough

Reorders a route-level error component declaration in an e2e Solid Start file and updates the code-splitter compiler to wrap exported Solid components in a getter for TDZ-safe access, plus suppresses inline warnings for Solid targets.

Changes

Cohort / File(s) Summary
E2E Test Route Declaration
e2e/solid-start/basic-solid-query/src/routes/posts.$postId.tsx		 PostErrorComponent({ error }: { error: any }) moved from above the Route declaration to below it; signature, export, and references unchanged.
Solid Framework Code-Splitting Logic
packages/router-plugin/src/core/code-splitter/compilers.ts		 When targetFramework is solid, exported component references are wrapped in a getter to avoid TDZ; function marks modified and returns early for that path. Inline warning emission now skips Solid targets (requires NODE_ENV !== production AND targetFramework !== solid to add warning).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer Code
  participant Compiler as Code-Splitter Compiler
  participant Output as Compiled Output

  note right of Compiler #DDEEFF: compileCodeSplitReferenceRoute

  Dev->>Compiler: provide route/component AST
  alt targetFramework == "solid" and identifier is exported
    Compiler-->>Compiler: wrap reference in getter (TDZ-safe)\nset modified = true\nreturn early
    Compiler->>Output: emit getter-wrapped export
  else
    Compiler->>Compiler: normal split handling
    Compiler->>Output: emit normal split code
  end
  Note over Output: Inline warnings only emitted when\nNODE_ENV != production AND targetFramework != "solid"
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review the getter-wrapping implementation and ensure it preserves original export semantics and consumer import expectations.
  • Verify early-return behavior doesn't skip other necessary transformations.
  • Confirm warning suppression condition is correct and intentional.

Possibly related PRs

Suggested labels

package: solid-router

Suggested reviewers

  • schiller-manuel

Poem

🐰 A tiny rabbit taps the code,
Hops a getter down the road.
TDZ fears chased away,
Error component moves to play,
Quiet builds and calmer load. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix(solid): fix hoisting for solid" is directly related to the main changes in the changeset. The PR addresses a temporal dead zone issue with hoisting in Solid by wrapping exported component references in getter functions to defer evaluation. The title is clear, concise, and accurately reflects the primary objective of fixing hoisting behavior for Solid applications.
Linked Issues Check ✅ Passed The code changes directly address the objectives outlined in linked issue #5646. The PR fixes the temporal dead zone issue [#5646] that caused the suspense-transition page to fail in dev server while working in production. The compiler changes wrap exported component references in getter functions for Solid targets [#5646], deferring evaluation until after component declarations, which resolves the ReferenceError that prevented proper rendering in development mode.
Out of Scope Changes Check ✅ Passed All code changes in the pull request are in scope and directly related to fixing the temporal dead zone issue for Solid. The modification to the e2e test file represents necessary updates to test the fix, while the compiler changes in compilers.ts implement the core solution by wrapping exported component references in getter functions specifically for Solid targets. No extraneous or unrelated changes are present in this changeset.
✨ 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(solid)--avoid-temporal-dead-zone
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6944cca and c6bbf59.

📒 Files selected for processing (1)
  • packages/router-plugin/src/core/code-splitter/compilers.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/router-plugin/src/core/code-splitter/compilers.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Preview
  • GitHub Check: Test

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

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

@nx-cloud
Copy link

nx-cloud bot commented Oct 27, 2025 •
edited
Loading

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.

View your CI Pipeline Execution ↗ for commit c6bbf59

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ❌ Failed 7m 58s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 23s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-27 14:25:14 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 27, 2025 •
edited
Loading

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5654

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5654

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5654

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5654

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5654

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5654

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5654

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5654

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5654

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5654

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5654

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5654

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5654

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5654

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5654

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5654

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5654

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5654

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5654

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5654

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5654

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5654

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5654

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@5654

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5654

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5654

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5654

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5654

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5654

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5654

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5654

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5654

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5654

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5654

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5654

commit: c6bbf59

coderabbitai[bot]
coderabbitai bot reviewed Oct 27, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5ac27b and 6944cca.

📒 Files selected for processing (2)
  • e2e/solid-start/basic-solid-query/src/routes/posts.$postId.tsx (1 hunks)
  • packages/router-plugin/src/core/code-splitter/compilers.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/router-plugin/src/core/code-splitter/compilers.ts
  • e2e/solid-start/basic-solid-query/src/routes/posts.$postId.tsx
packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/**

📄 CodeRabbit inference engine (AGENTS.md)

Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories

Files:

  • packages/router-plugin/src/core/code-splitter/compilers.ts
packages/router-plugin/**

📄 CodeRabbit inference engine (AGENTS.md)

Use unplugin for universal bundler plugins in the router-plugin package

Files:

  • packages/router-plugin/src/core/code-splitter/compilers.ts
**/src/routes/**

📄 CodeRabbit inference engine (AGENTS.md)

Place file-based routes under src/routes/ directories

Files:

  • e2e/solid-start/basic-solid-query/src/routes/posts.$postId.tsx
e2e/**

📄 CodeRabbit inference engine (AGENTS.md)

Store end-to-end tests under the e2e/ directory

Files:

  • e2e/solid-start/basic-solid-query/src/routes/posts.$postId.tsx
🧠 Learnings (1)
📓 Common learnings 
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Preview
  • GitHub Check: Test
🔇 Additional comments (2)
packages/router-plugin/src/core/code-splitter/compilers.ts (1)

454-463: LGTM: Appropriate safeguard for Solid module evaluation.

Skipping the inline warning for Solid builds prevents interference with module evaluation order, which is important given the TDZ concerns. The warning is still emitted to console at line 451, so developers are still notified.

e2e/solid-start/basic-solid-query/src/routes/posts.$postId.tsx (1)

13-16: Good demonstration of the TDZ fix, but depends on the critical fix in compilers.ts.

Moving PostErrorComponent below the Route declaration effectively demonstrates the temporal dead zone scenario. With the intended fix, the reference at line 9 should be wrapped in a getter to defer evaluation. However, this depends on the control flow issue in compilers.ts (lines 267-283) being resolved.

After addressing the critical issue in compilers.ts, verify that this reordering works correctly by testing that:

  1. The component reference is wrapped in a getter: errorComponent: () => PostErrorComponent
  2. No ReferenceError occurs at runtime
  3. The error component renders correctly when triggered

@birkskyum birkskyum added this to the catch up solid to react milestone Oct 27, 2025
@birkskyum birkskyum closed this Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

package: router-plugin

Projects

None yet

Milestone

catch up solid to react

Development

Successfully merging this pull request may close these issues.

solid-start - suspense-transition test only work on prod

2 participants

@birkskyum