Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR exposes bundler-specific Start plugin APIs via explicit subpath exports ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 28d9e51
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview5 package(s) bumped directly, 0 bumped as dependents. 🟨 Minor bumps
🟩 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: 2
🤖 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/src/plugin/vite.ts`:
- Around line 5-8: The type-only imports (TanStackStartViteInputConfig,
TanStackStartVitePluginCoreOptions) are currently placed before other imports
and violate the ESLint import/order rule; move the `import type {
TanStackStartViteInputConfig, TanStackStartVitePluginCoreOptions } from
'@tanstack/start-plugin-core/vite'` statement to the end of the import block
(after all builtin, external, internal, parent, sibling, index/object imports)
so type imports come last and ESLint passes.
In `@packages/start-plugin-core/src/index.ts`:
- Line 3: The root export change removed previously available Vite/RSBuild
exports and may break consumers; restore temporary compatibility by re-adding
deprecated re-exports in packages/start-plugin-core/src/index.ts for the
original symbols (e.g., the Vite/RSBuild-related exports that were removed)
alongside the existing export of START_ENVIRONMENT_NAMES, and mark them as
deprecated in comments and the changelog; alternatively, if you intentionally
removed them, add an explicit migration note in the PR and package release notes
documenting which symbols were removed and the recommended new import paths so
consumers can migrate safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ebd6721-d7c0-4abd-ade5-934fd756aba1
📒 Files selected for processing (20)
.changeset/optional-bundler-start-plugins.mdpackages/react-start-rsc/src/plugin/vite.tspackages/react-start/package.jsonpackages/react-start/src/plugin/rsbuild.tspackages/react-start/src/plugin/vite.tspackages/solid-start/package.jsonpackages/solid-start/src/plugin/rsbuild.tspackages/solid-start/src/plugin/vite.tspackages/start-plugin-core/package.jsonpackages/start-plugin-core/src/import-protection/utils.tspackages/start-plugin-core/src/index.tspackages/start-plugin-core/src/rsbuild/import-protection.tspackages/start-plugin-core/src/rsbuild/index.tspackages/start-plugin-core/src/utils.tspackages/start-plugin-core/src/vite/index.tspackages/start-plugin-core/tests/utils.test.tspackages/start-plugin-core/vite.config.tspackages/vue-start/package.jsonpackages/vue-start/src/plugin/rsbuild.tspackages/vue-start/src/plugin/vite.ts
Comment on lines
5
to
+8
| import type { | ||
| TanStackStartViteInputConfig, | ||
| TanStackStartVitePluginCoreOptions, | ||
| } from '@tanstack/start-plugin-core' | ||
| } from '@tanstack/start-plugin-core/vite' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and read its current state
find . -name "vite.ts" -path "*/react-start/src/plugin/*" | head -20Repository: TanStack/router
Length of output: 101
🏁 Script executed:
# Check file size and read the import section
if [ -f "packages/react-start/src/plugin/vite.ts" ]; then
wc -l packages/react-start/src/plugin/vite.ts
echo "---"
head -30 packages/react-start/src/plugin/vite.ts
fiRepository: TanStack/router
Length of output: 1157
🏁 Script executed:
# Look for ESLint configuration files
find . -maxdepth 3 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" -o -name "eslint.config.ts" \) 2>/dev/null | head -20Repository: TanStack/router
Length of output: 917
🏁 Script executed:
# Read the ESLint config for react-start
cat packages/react-start/eslint.config.jsRepository: TanStack/router
Length of output: 819
🏁 Script executed:
# Read the root ESLint config to find import/order rules
cat eslint.config.jsRepository: TanStack/router
Length of output: 819
🏁 Script executed:
# The root config uses `@tanstack/eslint-config` from npm
# Let me search for any import/order configuration hints or ESLint disable comments in the repo
rg "import/order" -A 5 -B 5 packages/Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Also check if there are any ESLint disable comments in vite.ts that might give hints
cat -n packages/react-start/src/plugin/vite.ts | head -20Repository: TanStack/router
Length of output: 782
🏁 Script executed:
# Let me check if there are any eslint disable comments in similar files
# or patterns with import/order in the codebase
rg "eslint.*import" -A 2 -B 2 packages/ | head -50Repository: TanStack/router
Length of output: 3328
🏁 Script executed:
# Check if there's a package.json or docs about ESLint import rules
grep -r "import/order" . 2>/dev/null | head -10Repository: TanStack/router
Length of output: 2158
🏁 Script executed:
# Verify the exact import groups by checking the config source
grep -A 20 "'import/order'" node_modules/.pnpm/@tanstack+eslint-config@*/node_modules/@tanstack/eslint-config/dist/import.js | head -25Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Let me check the actual import groups configuration more carefully
find node_modules -name "import.ts" -path "*tanstack/eslint-config*" -exec cat {} \;Repository: TanStack/router
Length of output: 945
Fix import ordering to satisfy ESLint import/order rule.
Type imports on lines 5-8 violate the import ordering configuration, which requires type imports to appear after all other import groups (builtin, external, internal, parent, sibling, index, object). Move both type import blocks to the end of the import section.
Proposed fix
import {
START_ENVIRONMENT_NAMES,
tanStackStartVite,
} from '@tanstack/start-plugin-core/vite'
-import type {
- TanStackStartViteInputConfig,
- TanStackStartVitePluginCoreOptions,
-} from '@tanstack/start-plugin-core/vite'
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/vite'
import type { PluginOption } from 'vite'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { | |
| TanStackStartViteInputConfig, | |
| TanStackStartVitePluginCoreOptions, | |
| } from '@tanstack/start-plugin-core' | |
| } from '@tanstack/start-plugin-core/vite' | |
| import { | |
| START_ENVIRONMENT_NAMES, | |
| tanStackStartVite, | |
| } from '@tanstack/start-plugin-core/vite' | |
| 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/vite' | |
| import type { PluginOption } from 'vite' |
🧰 Tools
🪛 ESLint
[error] 5-8: @tanstack/start-plugin-core/vite type import should occur after import of ./shared
(import/order)
🤖 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, The type-only
imports (TanStackStartViteInputConfig, TanStackStartVitePluginCoreOptions) are
currently placed before other imports and violate the ESLint import/order rule;
move the `import type { TanStackStartViteInputConfig,
TanStackStartVitePluginCoreOptions } from '@tanstack/start-plugin-core/vite'`
statement to the end of the import block (after all builtin, external, internal,
parent, sibling, index/object imports) so type imports come last and ESLint
passes.
| export type { TanStackStartRsbuildPluginCoreOptions } from './rsbuild/types' | ||
| export type { TanStackStartRsbuildInputConfig } from './rsbuild/schema' | ||
| export { tanStackStartRsbuild } from './rsbuild/plugin' | ||
| export { START_ENVIRONMENT_NAMES } from './constants' |
Contributor
There was a problem hiding this comment.
Root export contraction risks consumer breakage on a fix release.
If root-level Vite/RSBuild exports were intentionally removed, this can break existing imports from @tanstack/start-plugin-core. Please either keep temporary compatibility re-exports (deprecated) or add explicit migration/versioning communication in this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/start-plugin-core/src/index.ts` at line 3, The root export change
removed previously available Vite/RSBuild exports and may break consumers;
restore temporary compatibility by re-adding deprecated re-exports in
packages/start-plugin-core/src/index.ts for the original symbols (e.g., the
Vite/RSBuild-related exports that were removed) alongside the existing export of
START_ENVIRONMENT_NAMES, and mark them as deprecated in comments and the
changelog; alternatively, if you intentionally removed them, add an explicit
migration note in the PR and package release notes documenting which symbols
were removed and the recommended new import paths so consumers can migrate
safely.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/react-start/start-manifest/tests/start-manifest.spec.ts (1)
366-370: Consider centralizing repeated shared-widget border assertions.The same
expect.poll(... getBorderTopColor ..., { timeout: 5_000 })logic appears in three tests. A small helper would reduce duplication and keep timeout/expectation changes in one place.♻️ Optional refactor
+async function expectSharedWidgetBorder(page: Page) { + await expect + .poll(() => getBorderTopColor('shared-widget', page), { + timeout: 5_000, + }) + .toBe(SHARED_WIDGET_BORDER) +} ... - await expect - .poll(() => getBorderTopColor('shared-widget', page), { - timeout: 5_000, - }) - .toBe(SHARED_WIDGET_BORDER) + await expectSharedWidgetBorder(page)Also applies to: 409-413, 431-435
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/start-manifest/tests/start-manifest.spec.ts` around lines 366 - 370, Create a small test helper (e.g., assertSharedWidgetBorder or expectSharedWidgetBorder) that encapsulates the repeated expect.poll call: call getBorderTopColor('shared-widget', page) and assert .toBe(SHARED_WIDGET_BORDER) with a default timeout of 5_000 (configurable via an optional parameter). Replace the three inline usages (the expect.poll blocks currently using getBorderTopColor and SHARED_WIDGET_BORDER) with calls to this helper so timeout/expectation changes are centralized; keep helper signature accepting the Playwright Page (or same page variable) and an optional timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/react-start/start-manifest/tests/start-manifest.spec.ts`:
- Around line 366-370: Create a small test helper (e.g.,
assertSharedWidgetBorder or expectSharedWidgetBorder) that encapsulates the
repeated expect.poll call: call getBorderTopColor('shared-widget', page) and
assert .toBe(SHARED_WIDGET_BORDER) with a default timeout of 5_000 (configurable
via an optional parameter). Replace the three inline usages (the expect.poll
blocks currently using getBorderTopColor and SHARED_WIDGET_BORDER) with calls to
this helper so timeout/expectation changes are centralized; keep helper
signature accepting the Playwright Page (or same page variable) and an optional
timeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 916e4d98-1478-4e1f-b952-427532056679
📒 Files selected for processing (2)
e2e/react-start/import-protection/tests/import-protection.spec.tse2e/react-start/start-manifest/tests/start-manifest.spec.ts
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 #7247
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores