fix: unify virtual module handling for Start Vite plugins#7178
Merged
schiller-manuel merged 6 commits intomainfrom Apr 13, 2026
Merged
fix: unify virtual module handling for Start Vite plugins#7178schiller-manuel merged 6 commits intomainfrom
schiller-manuel merged 6 commits intomainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a shared Vite virtual-module factory ( Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Client)
participant Vite as Vite Dev Server
participant Plugin as Start Plugin Core (createVirtualModule)
participant Config as ConfigResolver/Entry Planner
Browser->>Vite: GET /@id/virtual:tanstack-start-client-entry
Vite->>Plugin: resolveId("virtual:tanstack-start-client-entry")
Plugin->>Config: getClientEntry()
Config-->>Plugin: client entry path
Plugin-->>Vite: resolved id ("\0virtual:tanstack-start-client-entry")
Vite->>Plugin: load(resolved id)
Plugin-->>Vite: module source (import "normalized/path/to/client")
Vite-->>Browser: module source (200)
Browser->>Vite: dynamic import → hydration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
View your CI Pipeline Execution ↗ for commit b8c884e
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview2 package(s) bumped directly, 3 bumped as dependents. 🟩 Patch bumps
|
Contributor
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/start-plugin-core/tests/createVirtualModule.test.ts (1)
1-38: LGTM! Good coverage of core virtual module ID resolution behavior.The tests effectively validate the
\0prefix for Vite virtual modules and the#→%23encoding for hash-prefixed module IDs. Theas anycasts are acceptable here for accessing plugin internals in test code.Consider adding edge case tests for non-matching IDs to verify the handler returns
undefined:💡 Optional: Additional edge case test
+ test('returns undefined for non-matching ids', () => { + const plugin = createVirtualModule({ + name: 'test-non-match', + moduleId: 'virtual:test-module', + load() { + return 'export const ok = true' + }, + }) + + expect((plugin as any).resolveId.handler('virtual:other-module')).toBeUndefined() + expect((plugin as any).load.handler('\0virtual:other-module')).toBeUndefined() + }),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/tests/createVirtualModule.test.ts` around lines 1 - 38, Add an edge-case test in createVirtualModule.test.ts that ensures resolveId.handler and load.handler return undefined for non-matching IDs: create another test using a plugin from createVirtualModule (e.g., moduleId 'virtual:test-module' and '#virtual:test-module') and call resolveId.handler and load.handler with unrelated strings (like 'other-module' and '\0other-module') asserting they return undefined to validate non-matching behavior.packages/start-plugin-core/src/vite/createVirtualModule.ts (1)
4-7: Consider using Vite's plugin context type instead ofanyfor better type safety.The
this: anytype loses compile-time safety for properties likethis.environment,this.error(), etc. that are used in the load handlers throughout this PR.💡 Optional: Stricter typing using Vite's plugin context
+import type { Plugin, PluginContext } from 'vite' + +// Vite's load handler receives a PluginContext-like object with environment info +interface VirtualModuleLoadContext extends PluginContext { + environment: { name: string; mode: string; config: { command: string; consumer?: string } } +} type VirtualModuleLoadHandler = ( - this: any, + this: VirtualModuleLoadContext, id: string, ) => string | null | undefined | Promise<string | null | undefined>This is optional since
anyprovides flexibility, but stricter types would catch misuse at compile time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/src/vite/createVirtualModule.ts` around lines 4 - 7, Replace the unsafe this: any in the VirtualModuleLoadHandler type with Vite's PluginContext to get proper typings for this.environment, this.error, etc.; import type { PluginContext } from 'vite' and update the type to use that symbol (e.g., this: PluginContext) so all load handlers using VirtualModuleLoadHandler gain compile-time safety while preserving behavior.packages/start-plugin-core/src/vite/plugins.ts (1)
20-22: Remove the redundant.replaceAll('\\', '/')afternormalizePath.Vite's
normalizePathalready normalizes all path separators to forward slashes. The additional.replaceAllcall has no effect and should be removed.♻️ Suggested simplification
load() { - return `import ${JSON.stringify(normalizePath(opts.getClientEntry()).replaceAll('\\', '/'))}` + return `import ${JSON.stringify(normalizePath(opts.getClientEntry()))}` },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/src/vite/plugins.ts` around lines 20 - 22, The load() function currently returns a string using normalizePath(opts.getClientEntry()).replaceAll('\\', '/'); remove the redundant .replaceAll('\\', '/') because normalizePath already converts backslashes to forward slashes; update the load() return expression to use only normalizePath(opts.getClientEntry()) and keep the JSON.stringify wrapping as-is so the module import path remains correct.
🤖 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/react-start-rsc/src/plugin/vite.ts`:
- Around line 3-9: Move the inline "type" specifiers out of the mixed import and
into a dedicated top-level type-only import: keep the runtime import for
createVirtualModule from '@tanstack/start-plugin-core' and add a separate
"import type { TanStackStartVitePluginCoreOptions,
ViteRscForwardSsrResolverStrategy } from '@tanstack/start-plugin-core'". Update
references to TanStackStartVitePluginCoreOptions and
ViteRscForwardSsrResolverStrategy accordingly so the file no longer uses "type"
inline specifiers in the mixed import.
In `@packages/react-start/src/plugin/vite.ts`:
- Around line 1-6: The current combined import mixes runtime values and types;
split into a runtime import for START_ENVIRONMENT_NAMES and tanStackStartVite
and a top-level type-only import for TanStackStartViteInputConfig and
TanStackStartVitePluginCoreOptions to satisfy the
import/consistent-type-specifier-style rule. Locate the import that currently
includes START_ENVIRONMENT_NAMES, tanStackStartVite, type
TanStackStartViteInputConfig, and type TanStackStartVitePluginCoreOptions and
change it so runtime symbols (START_ENVIRONMENT_NAMES, tanStackStartVite) are
imported normally and the two types are imported with an "import type" statement
at the top level.
In `@packages/solid-start/src/plugin/vite.ts`:
- Around line 1-6: The import statement mixes value and type specifiers;
separate the type-only imports into an `import type` clause: keep the runtime
values START_ENVIRONMENT_NAMES and tanStackStartVite in the original import from
'@tanstack/start-plugin-core' and move the type names
TanStackStartViteInputConfig and TanStackStartVitePluginCoreOptions into a new
`import type` statement so the code complies with the enforced type-specifier
lint rule.
In `@packages/start-plugin-core/src/vite/serialization-adapters-plugin.ts`:
- Line 18: The load handler's explicit this type annotation (`load(this: {
environment: { name: string } })`) narrows the Vite plugin context and must be
removed; update the `load` function signature in
`serialization-adapters-plugin.ts` to omit the `this` type (i.e., `load(...) {
... }`) so it uses the full Vite plugin `this` context used by
`createVirtualModule` and `opts.load.call(this, id)`—do not replace it with a
stricter custom type.
In `@packages/vue-start/src/plugin/vite.ts`:
- Around line 1-6: Move the two type-only imports into a top-level "import type"
statement: keep the runtime imports (START_ENVIRONMENT_NAMES, tanStackStartVite)
in the existing import block and move TanStackStartViteInputConfig and
TanStackStartVitePluginCoreOptions into a separate "import type" line so the
file imports look like runtime imports first and type imports second, satisfying
import/consistent-type-specifier-style while preserving usage of
START_ENVIRONMENT_NAMES and tanStackStartVite.
---
Nitpick comments:
In `@packages/start-plugin-core/src/vite/createVirtualModule.ts`:
- Around line 4-7: Replace the unsafe this: any in the VirtualModuleLoadHandler
type with Vite's PluginContext to get proper typings for this.environment,
this.error, etc.; import type { PluginContext } from 'vite' and update the type
to use that symbol (e.g., this: PluginContext) so all load handlers using
VirtualModuleLoadHandler gain compile-time safety while preserving behavior.
In `@packages/start-plugin-core/src/vite/plugins.ts`:
- Around line 20-22: The load() function currently returns a string using
normalizePath(opts.getClientEntry()).replaceAll('\\', '/'); remove the redundant
.replaceAll('\\', '/') because normalizePath already converts backslashes to
forward slashes; update the load() return expression to use only
normalizePath(opts.getClientEntry()) and keep the JSON.stringify wrapping as-is
so the module import path remains correct.
In `@packages/start-plugin-core/tests/createVirtualModule.test.ts`:
- Around line 1-38: Add an edge-case test in createVirtualModule.test.ts that
ensures resolveId.handler and load.handler return undefined for non-matching
IDs: create another test using a plugin from createVirtualModule (e.g., moduleId
'virtual:test-module' and '#virtual:test-module') and call resolveId.handler and
load.handler with unrelated strings (like 'other-module' and '\0other-module')
asserting they return undefined to validate non-matching behavior.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd152089-06da-4906-9253-954c82498789
📒 Files selected for processing (16)
.changeset/rare-pigs-smash.mdpackages/react-start-rsc/src/plugin/vite.tspackages/react-start/src/plugin/vite.tspackages/solid-start/src/plugin/vite.tspackages/start-plugin-core/package.jsonpackages/start-plugin-core/src/index.tspackages/start-plugin-core/src/vite/createVirtualModule.tspackages/start-plugin-core/src/vite/dev-server-plugin/plugin.tspackages/start-plugin-core/src/vite/plugin.tspackages/start-plugin-core/src/vite/plugins.tspackages/start-plugin-core/src/vite/serialization-adapters-plugin.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/src/vite/start-manifest-plugin/plugin.tspackages/start-plugin-core/tests/createVirtualModule.test.tspackages/start-plugin-core/tests/virtual-client-entry-plugin.test.tspackages/vue-start/src/plugin/vite.ts
💤 Files with no reviewable changes (1)
- packages/start-plugin-core/package.json
packages/start-plugin-core/src/vite/serialization-adapters-plugin.ts
Outdated
Show resolved
Hide resolved
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…gin.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/react-start/src/plugin/vite.ts (1)
5-8:⚠️ Potential issue | 🟡 MinorAdjust import order to satisfy
import/order.Line 5-8 currently violates the configured import ordering rule from static analysis. Move this type import below the
./sharedvalue import block.Proposed fix
import { START_ENVIRONMENT_NAMES, tanStackStartVite, } from '@tanstack/start-plugin-core' -import type { - TanStackStartViteInputConfig, - TanStackStartVitePluginCoreOptions, -} from '@tanstack/start-plugin-core' import { configureRsc, reactStartRscVitePlugin, } from '@tanstack/react-start-rsc/plugin/vite' import path from 'pathe' import { reactStartDefaultEntryPaths, reactStartPluginDir } from './shared' +import type { + TanStackStartViteInputConfig, + TanStackStartVitePluginCoreOptions, +} from '@tanstack/start-plugin-core' import type { PluginOption } from 'vite'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-start/src/plugin/vite.ts` around lines 5 - 8, Move the type-only import of TanStackStartViteInputConfig and TanStackStartVitePluginCoreOptions so it appears after the existing value import from './shared' to satisfy the import/order rule; locate the import statement currently bringing in those types and cut/paste it directly below the import that references './shared' (ensuring it remains a type import).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/react-start/src/plugin/vite.ts`:
- Around line 5-8: Move the type-only import of TanStackStartViteInputConfig and
TanStackStartVitePluginCoreOptions so it appears after the existing value import
from './shared' to satisfy the import/order rule; locate the import statement
currently bringing in those types and cut/paste it directly below the import
that references './shared' (ensuring it remains a type import).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9570a49-90e8-48fd-b09b-63abbd56abd7
📒 Files selected for processing (2)
packages/react-start/src/plugin/vite.tspackages/start-plugin-core/src/vite/serialization-adapters-plugin.ts
Fixed 3 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
✅ The fix from Nx Cloud was applied
We fixed the ESLint import/order violation in start-compiler-plugin/plugin.ts that was introduced when the PR added createVirtualModule and resolveViteId imports after the existing ./module-specifier import. Moving those two imports above ./module-specifier restores the correct order (parent-directory before same-directory), resolving the lint failure.
Tip
✅ We verified this fix by re-running @tanstack/start-plugin-core:test:eslint.
Suggested Fix changes
diff --git a/packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts b/packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts
index d48fa65a27..7ef0cecfcd 100644
--- a/packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts
+++ b/packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts
@@ -15,12 +15,12 @@ import {
import { loadModuleForViteCompiler } from '../../start-compiler/load-module'
import { generateServerFnResolverModule } from '../../start-compiler/server-fn-resolver-module'
import { cleanId } from '../../start-compiler/utils'
+import { createVirtualModule } from '../createVirtualModule'
+import { resolveViteId } from '../../utils'
import {
createViteDevServerFnModuleSpecifierEncoder,
decodeViteDevServerModuleSpecifier,
} from './module-specifier'
-import { createVirtualModule } from '../createVirtualModule'
-import { resolveViteId } from '../../utils'
import type { CompileStartFrameworkOptions } from '../../types'
import type {
GenerateFunctionIdFnOptional,
➡️ This fix was applied by manuel.schiller@caligano.de
🎓 Learn more about Self-Healing CI on nx.dev
Co-authored-by: schiller-manuel <schiller-manuel@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #6588
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
Tests