feat: add rsbuild support for TanStack Start #6623
Conversation
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…issues-2060 tanstack start rsbuild
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…on-refinement-1b7e Branch implementation refinement
Restore the PR GitHub Actions workflow to the main-branch behavior because the quality-audit PR changes were not needed for feat/rsbuild. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds RsBuild integration across Start: new rsbuild plugins, loaders, prerender/post-build tooling, server-fn manifest/resolver, bundler runner replacing direct Vite scripts, framework rsbuild plugin entries/exports, virtual module ID changes, CSS/test adjustments, and multiple package metadata/entry updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Runner as run-bundler.mjs
participant Bundler as RsBuild/Vite
participant PluginCore as TanStackStartRsbuildPluginCore
participant Compiler as StartCompiler (loader)
participant Server as Dev/Prod Server
Dev->>Runner: npm run dev / build / preview
Runner->>Bundler: select bundler (BUNDLER env) and forward command
Bundler->>PluginCore: load rsbuild plugins
PluginCore->>Compiler: invoke start-compiler-loader / route-tree-loader
Compiler->>PluginCore: emit server-fn metadata & transformed modules
PluginCore->>Bundler: produce virtual modules (manifest, server-fn resolver, injected scripts)
alt build + prerender
PluginCore->>Server: start built server artifact for prerender
Server->>PluginCore: serve routes => prerender writes HTML
end
Bundler-->>Dev: build/dev complete (artifacts + manifest)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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. Comment |
|
View your CI Pipeline Execution ↗ for commit 4ccdde4
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/start-server-core/src/server-functions-handler.ts (1)
167-183: 🛠️ Refactor suggestion | 🟠 MajorRedundant redirect checks and dead code in
isRedirectResponse.Two issues in this block:
getRedirectOptions(unwrapped)is computed at Line 167, but if it returns falsy,getRedirectOptions(unwrapped)is computed again at Line 177. Cache the result from Line 167.
Boolean(getRedirectOptions(unwrapped))at Line 177 is dead code:getRedirectOptionsreturnsundefinedwhen!isRedirect(value), so this expression can only betruewhenisRedirect(unwrapped)is alreadytrue. The condition simplifies to justisRedirect(unwrapped).Proposed simplification
const redirectOptions = getRedirectOptions(unwrapped) if (redirectOptions) { return Response.json( { ...redirectOptions, isSerializedRedirect: true }, { headers: getResponseHeaders(unwrapped) }, ) } if (isResponseLike(unwrapped)) { - const isRedirectResponse = - isRedirect(unwrapped) || Boolean(getRedirectOptions(unwrapped)) - if (isRedirectResponse) { + if (isRedirect(unwrapped)) { return unwrapped }
🤖 Fix all issues with AI agents
In @.github/workflows/autofix.yml:
- Around line 12-14: The workflow currently grants broad runner token
permissions (permissions: contents: write and pull-requests: write); change the
workflow-level permissions block to set contents: read (remove contents: write
and pull-requests: write) so the GITHUB_TOKEN only has read access; leave write
actions (pushing fixes/commenting) to the autofix.ci GitHub App and ensure the
permissions block references the symbol permissions with contents: read to
enforce least privilege.
In `@e2e/react-start/basic/rsbuild.config.ts`:
- Around line 1-7: Reorder the import statements so Node.js built-ins come
first: move the imports for node:path and node:url (path and fileURLToPath)
before external package imports like { defineConfig } from '@rsbuild/core', {
pluginReact } from '@rsbuild/plugin-react', and { tanstackStart } from
'@tanstack/react-start/plugin/rsbuild'; ensure symbols referenced in this file
(defineConfig, pluginReact, path, fileURLToPath, tanstackStart, isSpaMode,
isPrerender) are preserved and only the import order is changed to satisfy the
Node built-in-before-external rule.
In `@e2e/react-start/basic/scripts/run-bundler.mjs`:
- Around line 21-35: The run function's Promise currently listens only for the
child's 'close' event and can hang if spawn emits an 'error'; update the run
implementation (the run function that calls spawn and the child variable) to
attach a child.on('error', ...) handler that rejects the Promise with the error
(ensuring no double-resolve/reject race with the existing 'close' handler) and
cleanly propagates spawn errors to callers.
In `@packages/react-start/package.json`:
- Around line 109-119: The peerDependencies entry for "vite" is not marked
optional in peerDependenciesMeta, causing unnecessary warnings; update the
package.json so that under "peerDependenciesMeta" you add an object for "vite"
with "optional": true (mirroring how "@rsbuild/core" is marked) so the "vite"
peer dependency becomes optional for consumers — locate the "peerDependencies"
and "peerDependenciesMeta" blocks in packages/react-start/package.json and add
"vite": { "optional": true } to the meta object.
In `@packages/router-plugin/src/rspack.ts`:
- Around line 70-80: The public API surface is inconsistent across bundlers;
make webpack.ts and vite.ts match rspack.ts by exporting the same set of
symbols: add the function variants tanstackRouterAutoImport,
tanstackRouterCodeSplitter, tanstackRouterGenerator and the class-style
auto-import export (e.g., TanStackRouterWebpackAutoImport) to webpack.ts (and if
missing in vite.ts add corresponding TanStackRouterViteAutoImport or the chosen
class names), or alternatively remove the extra class/function duplicates from
rspack.ts—pick one consistent pattern and apply it across
TanStackRouterRspack/TanStackRouterWebpack/TanStackRouterVite plus their
Generator, CodeSplitter, AutoImport and function helpers so all bundles export
the same identifiers (e.g., TanStackRouter{Bundler},
TanStackRouterGenerator{Bundler}, TanStackRouterCodeSplitter{Bundler},
TanStackRouterAutoImport{Bundler}, tanstackRouterGenerator,
tanstackRouterCodeSplitter, tanstackRouterAutoImport, tanstackRouter).
In `@packages/start-client-core/src/client-rpc/serverFnFetcher.ts`:
- Around line 38-50: isResponseLike currently claims value is Response but only
duck-types status and headers.get; change it to narrow to a minimal interface
(e.g., ResponseLike with the exact members your downstream code uses such as
json, text, status, headers) or add explicit checks for the methods used before
narrowing so downstream calls (in serverFnFetcher where isResponseLike is
consumed at lines referenced) are safe; update the function signature from
"value is Response" to "value is ResponseLike" (or similar) and include checks
that headers.get, json, and text are functions if those are invoked later.
In `@packages/start-plugin-core/package.json`:
- Around line 96-104: Update the package.json peerDependenciesMeta so that the
"vite" peer dependency is marked optional like "@rsbuild/core": add a "vite": {
"optional": true } entry under "peerDependenciesMeta" to prevent peer dependency
warnings for users who only use rsbuild; ensure the JSON object includes this
key alongside the existing "@rsbuild/core" entry.
In `@packages/start-plugin-core/src/rsbuild/plugin.ts`:
- Around line 716-725: The code hardcodes outputFilename = 'server.js' which
breaks if rspack's output.filename changes; update the logic in plugin.ts around
where serverBuild/serverEntryPath are set to derive the actual emitted filename
from the build output (e.g., inspect the rspack build stats/compilation assets
or the configured output.filename pattern) or accept a configurable option for
the server entry filename; specifically, use the rspack compilation output
mapping to find the emitted server entry file in serverOutputDir (referencing
serverBuild, serverEntryPath, serverOutputDir, and outputFilename) and import
that resolved filename instead of assuming 'server.js'.
- Around line 598-606: The forEach callback currently uses a concise arrow (fn:
any) => fn(middlewares, context) which implicitly returns fn's value; change it
to a block-bodied function so it does not return a value (e.g., (fn: any) => {
fn(middlewares, context) }) or replace the forEach with an explicit loop; update
the invocation around existingSetupMiddlewares (the branch handling
Array.isArray(existingSetupMiddlewares)) to call each fn with middlewares and
context without returning values before calling setupMiddlewares(middlewares,
context).
In `@packages/start-plugin-core/src/rsbuild/post-server-build.ts`:
- Around line 38-50: The spread of startConfig.spa.prerender.headers can be
undefined and causes a TypeError; replace the optional-chaining or raw spread
with a nullish-coalescing fallback so the spread always gets an object—i.e.,
change ...(startConfig.spa.prerender.headers) to
...(startConfig.spa.prerender.headers ?? {}) in the startConfig.pages.push block
(the code that sets prerender.headers and adds HEADERS.TSS_SHELL) and make the
identical change in the other occurrence where startConfig.spa.prerender.headers
is spread.
In `@packages/start-plugin-core/src/rsbuild/prerender.ts`:
- Around line 233-241: The retry path is broken because addCrawlPageTask(page)
returns early due to seen.has(page.path); in the catch block inside the
prerender retry logic, remove page.path from the seen set before incrementing
retriesByPath and re-queueing so addCrawlPageTask can enqueue it again;
specifically, update the catch handling around addCrawlPageTask, referencing
seen, retriesByPath, addCrawlPageTask, and
prerenderOptions.retryCount/retryDelay to remove the path from seen, increment
retriesByPath, wait retryDelay, then call addCrawlPageTask(page).
- Around line 234-237: The retry log message is hardcoded to "in 500ms" but the
code uses prerenderOptions.retryDelay (which may be undefined); update the retry
logic in the block that checks retries < (prerenderOptions.retryCount ?? 0) so
the logger.warn prints the actual delay value (use a resolvedDelay =
prerenderOptions.retryDelay ?? 500 to mirror the intended default) and pass that
resolvedDelay into setTimeout; refer to the variables logger.warn,
prerenderOptions.retryDelay, and the retries check to locate and fix the code.
In `@packages/start-plugin-core/src/rsbuild/start-compiler-loader.ts`:
- Around line 28-29: The module-level Map compilers and object serverFnsById are
never cleared and can retain stale state across rebuilds; update the build
lifecycle (e.g., inside beforeRun or the start of each build cycle where
beforeRun currently clears the disk manifest) to explicitly clear these
in-memory caches: call compilers.clear() to reset the Map and reassign or delete
all keys from serverFnsById (or set serverFnsById = {} if scope allows) before
each build, ensuring any envName-based cached StartCompiler entries and
accumulated server function entries are removed between rebuilds.
In `@packages/start-plugin-core/src/rsbuild/start-compiler-plugin.ts`:
- Around line 174-185: The getManifest() function currently sets the
module-level cached variable to {} inside the catch, causing transient read
errors to be cached permanently; change the logic so cached is only assigned on
successful read/JSON.parse (using fs.readFileSync(manifestPath, 'utf-8') and
JSON.parse(raw)) and do not set cached in the catch block—simply return an empty
object (or undefined) on error so subsequent calls will retry reading the file;
reference getManifest, cached, manifestPath, fs.readFileSync and JSON.parse to
locate and update the code.
- Around line 76-93: In generateManifestModule, id and fn.functionName are
interpolated into a single-quoted JS literal unescaped; replace those raw
interpolations with safe JSON.stringify usage (e.g., JSON.stringify(id) for the
property key and JSON.stringify(fn.functionName) for the functionName value) so
any quotes/backslashes are escaped like fn.extractedFilename currently is; keep
the existing importer and conditional isClientReferenced logic
(includeClientReferencedCheck and fn.isClientReferenced) unchanged while
switching the string construction to use the JSON-encoded values.
In `@packages/start-plugin-core/src/rsbuild/start-router-plugin.ts`:
- Around line 71-79: The function resolveLoaderPath is duplicated; extract it
into a single shared utility module (e.g., create and export resolveLoaderPath
from a new util file) and replace the duplicate implementations by importing
that exported resolveLoaderPath in both locations that currently define it (the
copy in start-router-plugin.ts and the other copy). Ensure the new util
preserves the same logic (uses fileURLToPath(import.meta.url), path.resolve, and
fs.existsSync) and update the imports in the modules that referenced the local
function to import the shared symbol instead.
- Around line 17-69: moduleDeclaration logic is duplicated across
moduleDeclaration (in start-router-plugin.ts), buildRouteTreeModuleDeclaration
(in plugin.ts), and the equivalent in start-router-plugin/plugin.ts; extract
this into a single shared utility function (e.g., createModuleDeclaration) that
accepts the same parameters (startFilePath, routerFilePath, corePluginOpts,
generatedRouteTreePath) and reuse it from all three locations. Implement the
helper to contain getImportPath and the string-building logic currently inside
moduleDeclaration, export it, then replace the existing inline functions by
importing and calling the new helper from each file (ensure callers still
receive the same returned string shape and update any referenced names like
getRouter/startInstance/createStart to match the helper’s implementation).
In `@packages/start-server-core/src/server-functions-handler.ts`:
- Around line 398-411: The duck-typing in isResponseLike currently only asserts
headers?.get exists but later code (in handlers that call isResponseLike) calls
headers.set; update isResponseLike to also verify that the headers object
exposes a set function (e.g., check typeof headers?.set === 'function') so
objects that only implement get won't be treated as Response-like, and keep the
rest of the existing guards (status and headers presence) intact; locate the
guard in function isResponseLike and add the set check to the returned
predicate.
- Around line 324-340: The catch block redundantly calls
getRedirectOptions(error) twice; compute const redirectOptions =
getRedirectOptions(error) once at the start of the isResponseLike(error) branch
and reuse it for the server-fn serialized redirect branch and for computing
isRedirectResponse (use isRedirect(error) || Boolean(redirectOptions)); then
remove the duplicate getRedirectOptions call so the logic in the isResponseLike
catch path (the redirectOptions && isServerFn serialized Response,
isRedirectResponse check, and setting X_TSS_RAW_RESPONSE) all use the single
redirectOptions variable.
🧹 Nitpick comments (22)
packages/router-plugin/src/rspack.ts (1)
47-49: Missing JSDoc onTanStackRouterAutoImportRspack.
TanStackRouterGeneratorRspackandTanStackRouterCodeSplitterRspackboth have@exampleJSDoc blocks showing usage in an rsbuild config. Consider adding one here for consistency.📝 Suggested JSDoc
+/** + * `@example` + * ```ts + * export default defineConfig({ + * // ... + * tools: { + * rspack: { + * plugins: [TanStackRouterAutoImportRspack()], + * }, + * }, + * }) + * ``` + */ const TanStackRouterAutoImportRspack = /* `#__PURE__` */ createRspackPlugin( unpluginRouteAutoImportFactory, )e2e/react-start/basic/src/styles/app.css (1)
26-28: Blanketoutline: none !importantremoves keyboard focus indicators.Even in an e2e test app, removing all outlines via
.using-mouse *can mask accessibility regressions if e2e tests ever assert focus behavior. Consider using:focus-visibleinstead so keyboard focus indicators remain intact while hiding outlines for mouse clicks::focus:not(:focus-visible) { outline: none; }packages/start-plugin-core/src/start-compiler-plugin/handleCreateServerFn.ts (1)
491-530: Minor:globalHandlersAST node created but not reused ininitHandlers.Line 492 creates
globalHandlersas aMemberExpression, butinitHandlers(Lines 496-512) builds its own duplicateMemberExpressionnodes forglobalThis.__tssServerFnHandlers. Consider reusingglobalHandlersviat.cloneNode()to reduce AST duplication.Proposed simplification
const globalHandlers = t.memberExpression( t.identifier('globalThis'), t.identifier('__tssServerFnHandlers'), ) const initHandlers = t.expressionStatement( t.assignmentExpression( '=', - t.memberExpression( - t.identifier('globalThis'), - t.identifier('__tssServerFnHandlers'), - ), + t.cloneNode(globalHandlers), t.logicalExpression( '||', - t.memberExpression( - t.identifier('globalThis'), - t.identifier('__tssServerFnHandlers'), - ), + t.cloneNode(globalHandlers), t.arrayExpression([]), ), ), )packages/start-plugin-core/src/rsbuild/start-compiler-plugin.ts (4)
104-155: Substantial code duplication betweengenerateManifestModuleandgenerateManifestModuleFromFile.The
getServerFnByIdfunction body (fallback resolution withfnModule,action,serverFnMeta,globalThis.__tssServerFnHandlers) is duplicated nearly verbatim between the two generators (Lines 107–154 and Lines 188–248). Any future bug fix or behavior change would need to be applied in both places.Consider extracting the shared resolution logic into a helper string or a separate runtime utility that both code paths can reference.
Also applies to: 188-248
511-520: Static analysis flagged ReDoS risk from variable in regex — low risk here but worth sanitizing.
handlerVar(Line 515) is derived from a prior regex match constraining it to[A-Za-z_$][\w$]*(valid JS identifiers), so the practical ReDoS risk is minimal. However, defensively escaping the variable is low-cost and eliminates the static analysis warning.♻️ Escape handlerVar before regex construction
+ const escapeRegex = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') const exportMatch = scope.match( - new RegExp(`([A-Za-z_$][\\w$]*):\\(\\)=>${handlerVar}`) + new RegExp(`([A-Za-z_$][\\w$]*):\\(\\)=>${escapeRegex(handlerVar)}`) )
353-362: Asset content scanning for every unmatched server function may be slow on large builds.
findAssetMatch(Lines 353–362) reads and scans potentially every JS asset for each server function that doesn't have a direct module-graph match. This is O(N × M) where N = unmatched server fns and M = asset count, and each iteration may trigger filesystem reads (Line 346–348 fallback).Consider caching asset content reads (they're already partially used via
getAssetContent) or building an inverted index of tokens → asset names once per compilation.
39-42: Extensive use ofanytypes across rspack/webpack interop layer.
CompilationModule, the various function parameters (compiler: any,compilation: any,module: any,chunk: any), and helper functions all rely onany. While this is understandable given the untyped nature of rspack/webpack plugin APIs, consider using@rspack/coretypes or at minimum defining minimal structural interfaces for the accessed properties to catch shape mismatches at compile time.packages/start-plugin-core/src/rsbuild/prerender.ts (2)
42-48:validateAndNormalizePrerenderPagesis called twice on the same pages.Lines 45–48 normalize
startConfig.pagesand then Lines 122–125 insideprerenderPagesnormalize them again. The second call is redundant since the pages are already normalized and mutated in-place viastartConfig.pages.♻️ Remove the duplicate call inside prerenderPages
const routerBasePath = joinURL('/', startConfig.router.basepath ?? '') - - const routerBaseUrl = new URL(routerBasePath, 'http://localhost') - startConfig.pages = validateAndNormalizePrerenderPages( - startConfig.pages, - routerBaseUrl, - ) - startConfig.pages.forEach((page) => addCrawlPageTask(page))Also applies to: 119-125
97-110:extractLinkswon't capture links with explicit basepath or protocol-relative URLs.The regex on Line 104 only matches
hrefvalues starting with/or./. Links written as relative paths without./(e.g.,href="subpage") or with the full origin (e.g.,href="http://localhost/foo") won't be crawled. This may be acceptable for the intended use case, but it's worth noting.packages/solid-start/src/plugin/rsbuild.ts (1)
6-9: Consider importingRsbuildPluginfrom@rsbuild/coreinstead of defining a local type withany.The local type erases the
setupAPI parameter type. Since@rsbuild/coreis already declared as a peer dependency, importing the canonical type would provide compile-time safety for plugin implementations.That said, this avoids a hard import dependency at the cost of type safety, which may be intentional to keep the module loadable without
@rsbuild/coreinstalled.Suggested improvement
-type RsbuildPlugin = { - name: string - setup: (api: any) => void -} +import type { RsbuildPlugin } from '@rsbuild/core'As per coding guidelines, "Use TypeScript strict mode with extensive type safety for all code" (
**/*.{ts,tsx}).packages/vue-start/src/plugin/rsbuild.ts (1)
6-9: Consider sharing theRsbuildPlugintype across framework plugins.This identical type definition is duplicated in
react-start,solid-start, andvue-start. Consider exporting it from@tanstack/start-plugin-core/rsbuild(or colocating it there) to avoid drift. Also,setup: (api: any) => voidloses type safety — if@rsbuild/coreexposes a properRsbuildPlugintype, importing it (behind the optional peer dep) would be preferable.packages/start-plugin-core/src/rsbuild/index.ts (1)
1-3: Vite-named exports from an rsbuild barrel may cause confusion.
VITE_ENVIRONMENT_NAMESandresolveViteIdcarry Vite-specific naming but are exported from the rsbuild entry point. If these are intentionally shared across bundler backends, consider renaming to bundler-agnostic names (e.g.,ENVIRONMENT_NAMES,resolveModuleId) to avoid confusion for consumers.packages/start-plugin-core/src/rsbuild/route-tree-loader.ts (1)
3-8: Tighten loader context and options typing to comply with strict mode guidelines.The
this: anyandrouterConfig?: anybypass TypeScript strict mode.LoaderContextis available from@rspack/core(v1.2.2), andConfigis available from@tanstack/router-generator. Update the types to improve type safety per the strict mode requirement:Suggested improvement
+import type { Config } from '@tanstack/router-generator' +import type { LoaderContext } from '@rspack/core' import { getClientRouteTreeContent } from './route-tree-state' -export default function routeTreeLoader(this: any) { +export default function routeTreeLoader(this: LoaderContext) { const callback = this.async() - const options = this.getOptions() as { - routerConfig?: any + const options = this.getOptions() as { + routerConfig?: Config root?: string }packages/start-plugin-core/src/rsbuild/start-manifest-plugin.ts (2)
246-277: Rspack plugin usesanyfor compiler and stats — loses type safety.The
apply(compiler: any)andstats: anyparameters bypass TypeScript's type system entirely. While rspack types may not be readily available, consider importingCompilerandStatstypes from@rspack/coreif they're a dependency, or at minimum add a local interface for the subset of API used.As per coding guidelines, TypeScript strict mode with extensive type safety should be used.
279-311: Virtual module caches manifest permanently after first read — no invalidation path.The generated virtual module (lines 296-305) uses a module-scoped
cachedvariable. Once the manifest is read successfully, subsequent calls always return the stale cached value, even if the manifest file on disk changes. This is likely acceptable for production builds, but during development or watch-mode rebuilds this could serve outdated data.If this is intentionally production-only, a brief comment documenting that assumption would help future maintainers.
packages/start-plugin-core/src/rsbuild/start-router-plugin.ts (1)
98-122: Confusing control flow ingetRouteTreeFileFooter— conditional assignment is immediately overwritten.Lines 105-111 conditionally assign
routeTreeFileFooterfrom the original config, but line 112 unconditionally reassigns it. The original footer content is preserved via the spread on line 119, so the logic is functionally correct, but the intermediate assignment to the outerrouteTreeFileFootervariable is misleading.A clearer approach would use a local variable for the parsed original footer:
♻️ Suggested clarification
function getRouteTreeFileFooter() { if (routeTreeFileFooter) { return routeTreeFileFooter } const { startConfig, resolvedStartConfig } = getConfig() const ogRouteTreeFileFooter = startConfig.router.routeTreeFileFooter - if (ogRouteTreeFileFooter) { - if (Array.isArray(ogRouteTreeFileFooter)) { - routeTreeFileFooter = ogRouteTreeFileFooter - } else { - routeTreeFileFooter = ogRouteTreeFileFooter() - } - } - routeTreeFileFooter = [ + let existingFooter: Array<string> = [] + if (ogRouteTreeFileFooter) { + existingFooter = Array.isArray(ogRouteTreeFileFooter) + ? ogRouteTreeFileFooter + : ogRouteTreeFileFooter() + } + routeTreeFileFooter = [ moduleDeclaration({ generatedRouteTreePath: getGeneratedRouteTreePath(), corePluginOpts, startFilePath: resolvedStartConfig.startFilePath, routerFilePath: resolvedStartConfig.routerFilePath, }), - ...(routeTreeFileFooter ?? []), + ...existingFooter, ] return routeTreeFileFooter }packages/start-plugin-core/src/rsbuild/plugin.ts (3)
30-33:RsbuildPlugintype usesapi: any— consider narrowing.While the full rsbuild API type may be complex, defining at least the methods used (
modifyRsbuildConfig,onAfterBuild,onAfterStartProdServer,context) as a partial interface would provide better type safety across this file.As per coding guidelines, TypeScript strict mode with extensive type safety should be used.
100-119:mergeRspackConfigshallow-merges onlyplugins,module.rules, andresolve.alias— other nested keys may be silently dropped.If
basehas keys undermoduleother thanrules(e.g.,parser,generator), the spread...next?.modulewill overwrite them. The same applies toresolvekeys beyondalias(e.g.,extensions,mainFields).This is acceptable if the merge is only used for known, controlled configurations within this file, but it could cause subtle breakage if user-provided rsbuild configs include additional nested keys.
♻️ More robust merge for resolve
resolve: { ...base?.resolve, ...next?.resolve, alias: { ...(base?.resolve?.alias ?? {}), ...(next?.resolve?.alias ?? {}), }, + extensions: [ + ...(base?.resolve?.extensions ?? []), + ...(next?.resolve?.extensions ?? []), + ], },
406-418: Hardcoded monorepo pathpackages/start-client-core/dist/esmonly resolves in the monorepo workspace.Line 406-408 builds a path relative to
rootthat assumes the TanStack monorepo structure. For published packages consumed by end users, this path won't exist. TheexistsSyncguard (line 415) prevents a crash, and the regex fallback (line 411, pushed on line 418) provides the actual matching — so this is safe. But the dead code path is confusing.Consider adding a comment explaining this is a monorepo dev-time optimization, or remove the
existsSyncbranch entirely and rely solely on the regex.packages/start-plugin-core/src/rsbuild/start-compiler-loader.ts (3)
31-38: Sync I/O inappendServerFnsToManifestblocks the event loop.
fs.mkdirSyncandfs.appendFileSyncare used inside what is ultimately called from an async loader. While this is typically acceptable in build tooling, it could become a bottleneck with many server functions. ThemkdirSynccall on every append is also redundant after the first call.♻️ Optional: cache directory creation
+const createdDirs = new Set<string>() const appendServerFnsToManifest = ( manifestPath: string, data: Record<string, ServerFn>, ) => { if (!manifestPath || Object.keys(data).length === 0) return - fs.mkdirSync(path.dirname(manifestPath), { recursive: true }) + const dir = path.dirname(manifestPath) + if (!createdDirs.has(dir)) { + fs.mkdirSync(dir, { recursive: true }) + createdDirs.add(dir) + } fs.appendFileSync(manifestPath, `${JSON.stringify(data)}\n`) }
42-54:getTransformCodeFilterForEnvrecomputes patterns on every call.
KindDetectionPatternsandLookupKindsPerEnvare presumably static. This function is called for every file processed by the loader (viashouldTransformCode). Consider caching the result perenv:♻️ Simple memoization
+const transformCodeFilterCache = new Map<string, Array<RegExp>>() function getTransformCodeFilterForEnv(env: 'client' | 'server'): Array<RegExp> { + const cached = transformCodeFilterCache.get(env) + if (cached) return cached const validKinds = LookupKindsPerEnv[env] const patterns: Array<RegExp> = [] for (const [kind, pattern] of Object.entries(KindDetectionPatterns) as Array< [keyof typeof KindDetectionPatterns, RegExp] >) { if (validKinds.has(kind)) { patterns.push(pattern) } } + transformCodeFilterCache.set(env, patterns) return patterns }
114-151:resolveIdPromise may never settle ifresolverthrows synchronously.If the
resolverfunction (line 133) throws synchronously instead of calling the callback, the Promise will hang indefinitely. Wrapping the resolver call in a try/catch inside the Promise executor would make this more resilient:🛡️ Suggested defensive fix
return new Promise((resolve) => { const resolver = loaderContext.getResolve?.({ conditionNames: ['import', 'module', 'default'], }) ?? loaderContext.resolve - resolver(resolveContext, source, (err: Error | null, result?: string) => { + try { + resolver(resolveContext, source, (err: Error | null, result?: string) => { - if (!err && result) { - resolve(cleanId(result)) - return - } - try { - const resolved = require.resolve(source, { - paths: [ - baseContext, - loaderContext.rootContext || loaderContext.context, - ].filter(Boolean), - }) - resolve(cleanId(resolved)) - } catch { - resolve(null) - } - }) + if (!err && result) { + resolve(cleanId(result)) + return + } + try { + const resolved = require.resolve(source, { + paths: [ + baseContext, + loaderContext.rootContext || loaderContext.context, + ].filter(Boolean), + }) + resolve(cleanId(resolved)) + } catch { + resolve(null) + } + }) + } catch { + resolve(null) + } })
| function moduleDeclaration({ | ||
| startFilePath, | ||
| routerFilePath, | ||
| corePluginOpts, | ||
| generatedRouteTreePath, | ||
| }: { | ||
| startFilePath: string | undefined | ||
| routerFilePath: string | ||
| corePluginOpts: TanStackStartVitePluginCoreOptions | ||
| generatedRouteTreePath: string | ||
| }): string { | ||
| function getImportPath(absolutePath: string) { | ||
| let relativePath = path.relative( | ||
| path.dirname(generatedRouteTreePath), | ||
| absolutePath, | ||
| ) | ||
|
|
||
| if (!relativePath.startsWith('.')) { | ||
| relativePath = './' + relativePath | ||
| } | ||
|
|
||
| relativePath = relativePath.split(path.sep).join('/') | ||
| return relativePath | ||
| } | ||
|
|
||
| const result: Array<string> = [ | ||
| `import type { getRouter } from '${getImportPath(routerFilePath)}'`, | ||
| ] | ||
| if (startFilePath) { | ||
| result.push( | ||
| `import type { startInstance } from '${getImportPath(startFilePath)}'`, | ||
| ) | ||
| } else { | ||
| result.push( | ||
| `import type { createStart } from '@tanstack/${corePluginOpts.framework}-start'`, | ||
| ) | ||
| } | ||
| result.push( | ||
| `declare module '@tanstack/${corePluginOpts.framework}-start' { | ||
| interface Register { | ||
| ssr: true | ||
| router: Awaited<ReturnType<typeof getRouter>>`, | ||
| ) | ||
| if (startFilePath) { | ||
| result.push( | ||
| ` config: Awaited<ReturnType<typeof startInstance.getOptions>>`, | ||
| ) | ||
| } | ||
| result.push(` } | ||
| }`) | ||
|
|
||
| return result.join('\n') | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the plugin.ts file in the repository
fd -t f "plugin.ts" packages/start-plugin-coreRepository: TanStack/router
Length of output: 966
🏁 Script executed:
#!/bin/bash
# Extract the moduleDeclaration function from start-router-plugin.ts
cat -n packages/start-plugin-core/src/rsbuild/start-router-plugin.ts | sed -n '17,69p'Repository: TanStack/router
Length of output: 1750
🏁 Script executed:
#!/bin/bash
# Verify the file path and check for buildRouteTreeModuleDeclaration
fd -t f "plugin.ts" packages/start-plugin-core -x cat -n {} | grep -A 50 "buildRouteTreeModuleDeclaration"Repository: TanStack/router
Length of output: 4143
🏁 Script executed:
#!/bin/bash
# Check which plugin.ts file contains buildRouteTreeModuleDeclaration
grep -l "buildRouteTreeModuleDeclaration" packages/start-plugin-core/src/**/*.tsRepository: TanStack/router
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Get context on where moduleDeclaration is used in start-router-plugin.ts
rg "moduleDeclaration" packages/start-plugin-core/src/rsbuild/start-router-plugin.ts -B 2 -A 2Repository: TanStack/router
Length of output: 329
🏁 Script executed:
#!/bin/bash
# Get context on where buildRouteTreeModuleDeclaration is used in plugin.ts
rg "buildRouteTreeModuleDeclaration" packages/start-plugin-core/src/start-router-plugin/plugin.ts -B 2 -A 2Repository: TanStack/router
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the correct file with buildRouteTreeModuleDeclaration
rg "buildRouteTreeModuleDeclaration" packages/start-plugin-core/src/rsbuild/plugin.ts -B 2 -A 2Repository: TanStack/router
Length of output: 425
🏁 Script executed:
#!/bin/bash
# Get the full content of both files to understand the context
echo "=== start-router-plugin.ts (lines 1-80) ==="
head -80 packages/start-plugin-core/src/rsbuild/start-router-plugin.ts
echo ""
echo "=== rsbuild/plugin.ts (lines 40-95) ==="
sed -n '40,95p' packages/start-plugin-core/src/rsbuild/plugin.tsRepository: TanStack/router
Length of output: 4004
🏁 Script executed:
#!/bin/bash
# Check where moduleDeclaration is called in start-router-plugin.ts
rg "moduleDeclaration\(" packages/start-plugin-core/src/rsbuild/start-router-plugin.ts -B 5 -A 2Repository: TanStack/router
Length of output: 610
🏁 Script executed:
#!/bin/bash
# Check where buildRouteTreeModuleDeclaration is called in plugin.ts
rg "buildRouteTreeModuleDeclaration\(" packages/start-plugin-core/src/rsbuild/plugin.ts -B 2 -A 5Repository: TanStack/router
Length of output: 605
🏁 Script executed:
#!/bin/bash
# Check if there are any other similar functions in the codebase
rg "declare module '@tanstack/.*-start'" packages/start-plugin-core/src --type ts -lRepository: TanStack/router
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check if buildRouteTreeModuleDeclaration is exported or could be imported
head -20 packages/start-plugin-core/src/rsbuild/plugin.tsRepository: TanStack/router
Length of output: 810
🏁 Script executed:
#!/bin/bash
# Check if there's a third duplicate in start-router-plugin/plugin.ts
rg "function.*ModuleDeclaration|function moduleDeclaration" packages/start-plugin-core/src/start-router-plugin/plugin.ts -B 2 -A 30Repository: TanStack/router
Length of output: 888
🏁 Script executed:
#!/bin/bash
# Check if buildRouteTreeModuleDeclaration is exported from plugin.ts
grep -A 2 "export.*buildRouteTreeModuleDeclaration\|export.*moduleDeclaration" packages/start-plugin-core/src/rsbuild/plugin.tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check file sizes and structure of all three files to understand the context better
echo "=== start-router-plugin.ts structure ==="
wc -l packages/start-plugin-core/src/rsbuild/start-router-plugin.ts
grep "^function\|^export\|^const.*=" packages/start-plugin-core/src/rsbuild/start-router-plugin.ts | head -20
echo ""
echo "=== rsbuild/plugin.ts structure ==="
wc -l packages/start-plugin-core/src/rsbuild/plugin.ts
grep "^function\|^export\|^const.*=" packages/start-plugin-core/src/rsbuild/plugin.ts | head -20
echo ""
echo "=== start-router-plugin/plugin.ts structure ==="
wc -l packages/start-plugin-core/src/start-router-plugin/plugin.ts
grep "^function\|^export\|^const.*=" packages/start-plugin-core/src/start-router-plugin/plugin.ts | head -20Repository: TanStack/router
Length of output: 1044
🏁 Script executed:
#!/bin/bash
# Verify the exact differences in parameter handling between the two functions
echo "=== moduleDeclaration parameter signature ==="
sed -n '17,27p' packages/start-plugin-core/src/rsbuild/start-router-plugin.ts
echo ""
echo "=== buildRouteTreeModuleDeclaration parameter signature ==="
sed -n '44,49p' packages/start-plugin-core/src/rsbuild/plugin.tsRepository: TanStack/router
Length of output: 595
🏁 Script executed:
#!/bin/bash
# Get the moduleDeclaration function from start-router-plugin/plugin.ts to confirm triple duplication
sed -n '/^function moduleDeclaration/,/^}/p' packages/start-plugin-core/src/start-router-plugin/plugin.ts | head -60Repository: TanStack/router
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Get line numbers of moduleDeclaration in start-router-plugin/plugin.ts
rg -n "^function moduleDeclaration" packages/start-plugin-core/src/start-router-plugin/plugin.tsRepository: TanStack/router
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Get the full moduleDeclaration from start-router-plugin/plugin.ts
sed -n '44,100p' packages/start-plugin-core/src/start-router-plugin/plugin.tsRepository: TanStack/router
Length of output: 1636
🏁 Script executed:
#!/bin/bash
# Confirm all three functions generate identical logic - compare key parts
echo "=== Checking all three module declaration functions ==="
echo "1. start-router-plugin.ts (rsbuild):"
sed -n '42,69p' packages/start-plugin-core/src/rsbuild/start-router-plugin.ts | tail -20
echo ""
echo "2. rsbuild/plugin.ts:"
sed -n '61,88p' packages/start-plugin-core/src/rsbuild/plugin.ts | tail -20
echo ""
echo "3. start-router-plugin/plugin.ts:"
sed -n '57,85p' packages/start-plugin-core/src/start-router-plugin/plugin.ts | tail -20Repository: TanStack/router
Length of output: 1666
Extract duplicated module declaration logic into a shared utility.
The moduleDeclaration function here is duplicated across three locations in the codebase:
packages/start-plugin-core/src/rsbuild/start-router-plugin.ts(lines 17-69)packages/start-plugin-core/src/rsbuild/plugin.tsasbuildRouteTreeModuleDeclaration(lines 44-88)packages/start-plugin-core/src/start-router-plugin/plugin.ts(lines 32-81)
All three generate identical TypeScript module augmentation declarations with the same logic for computing import paths, building conditional imports, and generating the Register interface. Create a shared utility function (e.g., in a common helpers file) and import it from all three locations.
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/start-router-plugin.ts` around lines
17 - 69, moduleDeclaration logic is duplicated across moduleDeclaration (in
start-router-plugin.ts), buildRouteTreeModuleDeclaration (in plugin.ts), and the
equivalent in start-router-plugin/plugin.ts; extract this into a single shared
utility function (e.g., createModuleDeclaration) that accepts the same
parameters (startFilePath, routerFilePath, corePluginOpts,
generatedRouteTreePath) and reuse it from all three locations. Implement the
helper to contain getImportPath and the string-building logic currently inside
moduleDeclaration, export it, then replace the existing inline functions by
importing and calling the new helper from each file (ensure callers still
receive the same returned string shape and update any referenced names like
getRouter/startInstance/createStart to match the helper’s implementation).
| function resolveLoaderPath(relativePath: string) { | ||
| const currentDir = path.dirname(fileURLToPath(import.meta.url)) | ||
| const basePath = path.resolve(currentDir, relativePath) | ||
| const jsPath = `${basePath}.js` | ||
| const tsPath = `${basePath}.ts` | ||
| if (fs.existsSync(jsPath)) return jsPath | ||
| if (fs.existsSync(tsPath)) return tsPath | ||
| return jsPath | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate: resolveLoaderPath is also duplicated in plugin.ts (lines 173-181).
Identical implementation in both files. Extract to a shared utility.
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/start-router-plugin.ts` around lines
71 - 79, The function resolveLoaderPath is duplicated; extract it into a single
shared utility module (e.g., create and export resolveLoaderPath from a new util
file) and replace the duplicate implementations by importing that exported
resolveLoaderPath in both locations that currently define it (the copy in
start-router-plugin.ts and the other copy). Ensure the new util preserves the
same logic (uses fileURLToPath(import.meta.url), path.resolve, and
fs.existsSync) and update the imports in the modules that referenced the local
function to import the shared symbol instead.
Re-run CI after an external pkg.pr.new Cloudflare worker error in the Preview publish step. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve branch-introduced review findings by hardening response/redirect handling, fixing prerender retry behavior, tightening workflow and peer dependency metadata, and improving rsbuild/webpack/vite plugin consistency. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/start-client-core/src/client-rpc/serverFnFetcher.ts (1)
332-339:⚠️ Potential issue | 🟠 MajorType mismatch:
processServerFnResponseparameter expectsResponsebut receivesResponseLike.At line 276,
processServerFnResponseis called withresponsetyped asResponseLike(line 221), but the function parameter (line 337) is typed asResponse. SinceResponseLikeisPick<Response, 'status' | 'ok' | 'headers' | 'body' | 'json' | 'text'>, it's a structural subset and not assignable to the fullResponsetype under strict TypeScript mode.Update the parameter type to
ResponseLikefor consistency:Proposed fix
async function processServerFnResponse({ response, onMessage, onError, }: { - response: Response + response: ResponseLike onMessage: (msg: any) => any onError?: (msg: string, error?: any) => void }) {
🤖 Fix all issues with AI agents
In `@packages/start-plugin-core/src/rsbuild/plugin.ts`:
- Around line 386-394: The synchronous deletion using
fs.rmSync(generatedRouteTreePath) can race with concurrent generators; instead,
perform a safe, non-racing removal: attempt an atomic rename of
generatedRouteTreePath to a temp/backup name (e.g., generatedRouteTreePath +
'.stale') inside a try/catch and only unlink the backup afterwards, or at
minimum replace fs.rmSync with an unlink/rename wrapped in try/catch that
ignores ENOENT/EBUSY errors; update the code around generatedRouteTreePath /
existingTree / registerDeclaration to use this safe rename-then-remove or
try/catch-unlink pattern rather than calling fs.rmSync directly.
- Around line 558-590: The dev middleware inside setupMiddlewares mutates
req.url by assigning joinURL(resolvedStartConfig.viteAppBase, req.url ?? '/'),
which can break downstream middleware; instead create a new request object with
the modified URL (e.g., construct a new Request/NodeRequest using the composed
URL) and pass that to serverBuild.fetch while leaving the original req.url
untouched, updating the NodeRequest construction site (where NodeRequest({ req,
res }) is created) to use the new URL and returning sendNodeResponse(res,
webRes) as before; apply the same non-mutating change to the analogous
production middleware to keep behavior consistent.
In `@packages/start-plugin-core/src/rsbuild/prerender.ts`:
- Around line 86-95: The catch in the prerendering block swallows errors from
prerenderPages (and upstream addCrawlPageTask/queue.start), so update the catch
in the async block around prerenderPages to log the error via
logger.error(error) and then rethrow the error so callers (e.g.,
postServerBuildRsbuild) observe the failure; keep the existing logging but add a
`throw error` after it to propagate the failure.
- Around line 186-189: The code computing isSpaShell risks a TypeError because
it uses optional chaining on startConfig.spa but then accesses
.prerender.outputPath without guarding against prerender being undefined; update
the check in the isSpaShell assignment to safely access prerender (e.g., use
startConfig.spa?.prerender?.outputPath or check startConfig.spa?.prerender
before reading outputPath) so comparison with cleanPagePath is only performed
when outputPath is defined, preserving the existing isSpaShell logic and
variable names.
- Line 50: The code sets process.env.TSS_PRERENDERING = 'true' but never resets
it and also swallows errors; modify the prerender flow (the function that sets
process.env.TSS_PRERENDERING in
packages/start-plugin-core/src/rsbuild/prerender.ts) to ensure the env var is
restored (delete or reset to previous value) in a finally block after
prerendering completes, and change the catch block to rethrow the caught error
so failures are propagated.
In `@packages/start-plugin-core/src/rsbuild/start-compiler-loader.ts`:
- Around line 163-182: The loadModule function currently short-circuits relative
ids (cleaned.startsWith('.') or '/') and calls fsp.readFile with cleaned, which
reads from process.cwd(); instead, always call resolveId(loaderContext, cleaned)
to get a proper module path (use ((await resolveId(loaderContext, cleaned)) ??
cleaned) only as a fallback for already-absolute paths), then skip if the
resolved result is falsy or contains '\0', and finally use the resolvedPath for
fsp.readFile and compiler.ingestModule; update the logic around cleanId,
resolveId, resolvedPath, fsp.readFile, and compiler.ingestModule accordingly so
relative imports are resolved relative to the importer rather than the CWD.
In `@packages/start-plugin-core/src/rsbuild/start-compiler-plugin.ts`:
- Around line 254-270: The plugin currently only hooks compiler.hooks.beforeRun
so resetServerFnCompilerState() and the tempManifestPath removal (fsp.rm) won’t
run in watch mode; update createServerFnManifestRspackPlugin to also tapPromise
into compiler.hooks.watchRun (using the same callback) so both
resetServerFnCompilerState() and await fsp.rm(tempManifestPath, { force: true })
run for watch builds as well, referencing the existing beforeRun handler logic
to avoid duplicating behavior.
🧹 Nitpick comments (15)
packages/router-plugin/src/vite.ts (1)
37-37: Consider adding a@deprecatednotice ontanStackRouterCodeSplitter.The new
tanstackRouterCodeSplitteralias aligns naming with the rest of the exports (tanstackRouter,tanstackRouterGenerator,tanstackRouterAutoImport) — nice cleanup. However, unlikeTanStackRouterVite(Line 51-52), the oldtanStackRouterCodeSplitterdoesn't carry a@deprecatedJSDoc. Adding one would guide consumers toward the consistent name.Suggested change
+/** + * `@deprecated` Use `tanstackRouterCodeSplitter` instead. + */ const tanStackRouterCodeSplitter = createVitePlugin( unpluginRouterCodeSplitterFactory, )Also applies to: 60-61
packages/router-plugin/src/webpack.ts (1)
36-38: Missing JSDoc@exampleblock for consistency with sibling plugins.
TanStackRouterGeneratorWebpack(line 10) andTanStackRouterCodeSplitterWebpack(line 23) both have@exampleJSDoc blocks showing usage.TanStackRouterAutoImportWebpackis missing one.📝 Suggested JSDoc addition
+/** + * `@example` + * ```ts + * export default { + * // ... + * plugins: [TanStackRouterAutoImportWebpack()], + * } + * ``` + */ const TanStackRouterAutoImportWebpack = /* `#__PURE__` */ createWebpackPlugin( unpluginRouteAutoImportFactory, )packages/start-client-core/src/client-rpc/serverFnFetcher.ts (1)
71-85:redirect(payload as any)bypasses type checking on the redirect payload.If the shape of
payloaddoesn't match whatredirect()expects beyondisSerializedRedirect, this will silently produce a malformed redirect. Consider narrowing to at least validate thatpayloadhashrefor the required redirect fields before callingredirect().packages/start-plugin-core/src/rsbuild/start-compiler-loader.ts (4)
120-161: Promise may hang if the resolver callback is never invoked.If
resolver(...)neither throws nor invokes its callback (e.g., a buggy or custom resolver in the rspack configuration), this promise will never settle, silently stalling the build. Consider wrapping it with a timeout or anAbortSignal-based guard.🛡️ Suggested defensive timeout
return new Promise((resolve) => { + const timeout = setTimeout(() => { + resolve(null) + }, 10_000) const resolver = loaderContext.getResolve?.({ conditionNames: ['import', 'module', 'default'], }) ?? loaderContext.resolve try { resolver(resolveContext, source, (err: Error | null, result?: string) => { + clearTimeout(timeout) if (!err && result) { resolve(cleanId(result)) return } try { const resolved = require.resolve(source, { paths: [ baseContext, loaderContext.rootContext || loaderContext.context, ].filter(Boolean), }) resolve(cleanId(resolved)) } catch { resolve(null) } }) } catch { + clearTimeout(timeout) resolve(null) } })
48-60:getTransformCodeFilterForEnvallocates a new array on every loader call.
shouldTransformCode→getTransformCodeFilterForEnvis called for every file processed by the loader. SinceLookupKindsPerEnvandKindDetectionPatternsare module-level constants, the result perenvis always identical. Memoizing byenvavoids repeated iteration and allocation.♻️ Optional: memoize by env
+const transformCodeFilterCache = new Map<string, Array<RegExp>>() + function getTransformCodeFilterForEnv(env: 'client' | 'server'): Array<RegExp> { + const cached = transformCodeFilterCache.get(env) + if (cached) return cached const validKinds = LookupKindsPerEnv[env] const patterns: Array<RegExp> = [] for (const [kind, pattern] of Object.entries(KindDetectionPatterns) as Array< [keyof typeof KindDetectionPatterns, RegExp] >) { if (validKinds.has(kind)) { patterns.push(pattern) } } + transformCodeFilterCache.set(env, patterns) return patterns }Also applies to: 115-118
201-233: Compiler instance is cached byenvNamewith first-call options — subsequent calls with different options are silently ignored.If a loader call for the same
envNamearrives with a differentenv,root, orframework(e.g., due to misconfiguration), the cached compiler from the first invocation is silently reused. This is likely fine in practice, but there's no validation or assertion guarding this invariant.Consider adding a debug-mode assertion (e.g., checking
compiler.options.env === env) after the cache hit to catch misconfiguration early.
184-186: Apply proper types from@rsbuild/coreto improve type safety for the loader context.The
this: anytyping at line 184 andloaderContext: anyparameters (lines 121, 165) bypass TypeScript checks entirely. Since@rsbuild/coreis a peer dependency and re-exports theRspacknamespace, useRspack.LoaderContext<LoaderOptions>for proper typing:Example fix
import type { Rspack } from '@rsbuild/core' async function resolveId( loaderContext: Rspack.LoaderContext<LoaderOptions>, source: string, importer?: string, ): Promise<string | null> { // ... } export default function startCompilerLoader( this: Rspack.LoaderContext<LoaderOptions>, code: string, map: any, ) { // ... }This aligns with the coding guideline requirement for "TypeScript strict mode with extensive type safety for all code."
packages/start-plugin-core/src/rsbuild/start-compiler-plugin.ts (2)
328-365: Consider caching asset content to avoid repeated I/O in the manifest resolution loop.
getAssetContentis called per-server-function per-asset in the fallback paths offindAssetMatch,findExportName, andfindModuleIdByFunctionId. For a project with N server functions and M assets, this can result in O(N×M) redundant reads of the same assets. Since this runs inside a singleafterEmitcallback, asset content is immutable at that point.♻️ Proposed fix — add a simple content cache
+ const assetContentCache = new Map<string, string | undefined>() const getAssetContent = async (assetName: string) => { + if (assetContentCache.has(assetName)) { + return assetContentCache.get(assetName) + } const assetFromCompilation = ... try { const assetPath = path.join(opts.serverOutputDir, assetName) - return await fsp.readFile(assetPath, 'utf-8') + const content = await fsp.readFile(assetPath, 'utf-8') + assetContentCache.set(assetName, content) + return content } catch { + assetContentCache.set(assetName, undefined) return undefined } }Apply the same caching pattern at the function level, setting
assetContentCachefor all code paths before returning.
42-45: LooseCompilationModuletype with index signature.The
[key: string]: anyindex signature meansmodule.idreturnsanyinstead ofstring | number | undefined, defeating type safety for property access. Consider using a more specific intersection or a mapped type if feasible, though this is understandable for rspack interop.packages/start-plugin-core/src/rsbuild/plugin.ts (3)
406-418: Hardcoded monorepo-relative path forstart-client-core.
path.resolve(root, 'packages/start-client-core/dist/esm')on line 406 only resolves in the TanStack Router monorepo. For consumer projects, this path will never exist. TheexistsSyncguard (line 415) prevents a runtime error, and the regex on line 410–411 covers the installed-package case, so this isn't a bug — but the hardcoded path is dead code outside the monorepo and could confuse future maintainers.Consider adding a brief comment explaining this is a monorepo development convenience, or deriving the path from
require.resolve/import.meta.resolve.
100-119:mergeRspackConfigsilently drops nested properties frombasewhennextdefines the same top-level key.The shallow spread
...nextat line 103 replaces all base keys not explicitly deep-merged. Currently onlyplugins,module.rules, andresolve.aliasare deep-merged. If bothbaseandnextsupplyoutputorexperiments, the base values are completely lost.This appears intentional for the current usage (the plugin sets complete configs), but it's a latent footgun for future callers. A brief JSDoc noting the shallow-merge behavior would help.
30-33: LocalRsbuildPlugintype usesanyfor theapiparameter.This loses all type safety on the rsbuild API calls throughout the file.
@rsbuild/coreis available as a dependency (>=1.0.0) and should provide proper types. Consider importing the realRsbuildPlugintype. If not suitable, at minimum type the most-used API methods (modifyRsbuildConfig,onAfterBuild,onAfterStartProdServer,context) as a lightweight interface.packages/start-plugin-core/src/rsbuild/prerender.ts (3)
119-125: Duplicate page validation — pages are validated twice.
routerBasePath,routerBaseUrl, andvalidateAndNormalizePrerenderPagesare computed and applied identically on lines 42–48 (inprerender()) and again on lines 119–125 (inprerenderPages()). The second pass is redundant sincestartConfig.pageswas already normalized beforeprerenderPagesis called.♻️ Remove the redundant validation
async function prerenderPages({ outputDir }: { outputDir: string }) { const seen = new Set<string>() const prerendered = new Set<string>() const retriesByPath = new Map<string, number>() const concurrency = startConfig.prerender?.concurrency ?? os.cpus().length logger.info(`Concurrency: ${concurrency}`) const queue = new Queue({ concurrency }) - const routerBasePath = joinURL('/', startConfig.router.basepath ?? '') - - const routerBaseUrl = new URL(routerBasePath, 'http://localhost') - startConfig.pages = validateAndNormalizePrerenderPages( - startConfig.pages, - routerBaseUrl, - )Then reference the outer
routerBasePathvariable insideprerenderPages. Currently it's re-declared (line 119 shadows line 42).
127-127: Biome:forEachcallback implicitly returns a value.
addCrawlPageTaskreturnsvoid, so this is functionally harmless, but using a block body satisfies the linter.♻️ Proposed fix
- startConfig.pages.forEach((page) => addCrawlPageTask(page)) + startConfig.pages.forEach((page) => { + addCrawlPageTask(page) + })
97-110: Link extraction regex doesn't handle self-closing anchors or single-quoted/unquoted attributes correctly beyond the basic case.The regex
/<a[^>]+href=["']([^"']+)["'][^>]*>/gconflates single and double quotes in its character class —[^"']matches up to whichever quote type appears first rather than the one that opened the attribute. For example,<a href="it's">would incorrectly truncate at the apostrophe.This is a minor edge case for prerender crawling, and HTML from SSR is likely well-formed, but worth noting.
| if (fs.existsSync(generatedRouteTreePath)) { | ||
| const existingTree = fs.readFileSync( | ||
| generatedRouteTreePath, | ||
| 'utf-8', | ||
| ) | ||
| if (!existingTree.includes(registerDeclaration)) { | ||
| fs.rmSync(generatedRouteTreePath) | ||
| } | ||
| } |
There was a problem hiding this comment.
Deleting the generated route tree file could race with other processes.
fs.rmSync(generatedRouteTreePath) on line 392 deletes the route tree file synchronously if it lacks the register declaration. If the router plugin or a watcher is concurrently regenerating this file, the delete could cause a race condition or a confusing error.
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/plugin.ts` around lines 386 - 394, The
synchronous deletion using fs.rmSync(generatedRouteTreePath) can race with
concurrent generators; instead, perform a safe, non-racing removal: attempt an
atomic rename of generatedRouteTreePath to a temp/backup name (e.g.,
generatedRouteTreePath + '.stale') inside a try/catch and only unlink the backup
afterwards, or at minimum replace fs.rmSync with an unlink/rename wrapped in
try/catch that ignores ENOENT/EBUSY errors; update the code around
generatedRouteTreePath / existingTree / registerDeclaration to use this safe
rename-then-remove or try/catch-unlink pattern rather than calling fs.rmSync
directly.
| const setupMiddlewares = ( | ||
| middlewares: Array<any>, | ||
| context: { environments?: Record<string, any> }, | ||
| ) => { | ||
| if (startConfig.vite?.installDevServerMiddleware === false) { | ||
| return | ||
| } | ||
| const serverEnv = | ||
| context.environments?.[VITE_ENVIRONMENT_NAMES.server] | ||
| middlewares.push(async (req: any, res: any, next: any) => { | ||
| if (res.headersSent || res.writableEnded) { | ||
| return next() | ||
| } | ||
| if (!serverEnv?.loadBundle) { | ||
| return next() | ||
| } | ||
| try { | ||
| const serverBundle = await serverEnv.loadBundle() | ||
| const serverBuild = serverBundle?.default ?? serverBundle | ||
| if (!serverBuild?.fetch) { | ||
| return next() | ||
| } | ||
| req.url = joinURL( | ||
| resolvedStartConfig.viteAppBase, | ||
| req.url ?? '/', | ||
| ) | ||
| const webReq = new NodeRequest({ req, res }) | ||
| const webRes = await serverBuild.fetch(webReq) | ||
| return sendNodeResponse(res, webRes) | ||
| } catch (error) { | ||
| return next(error) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Dev middleware mutates req.url — may break downstream middleware.
Line 580–583 modifies req.url in place by prepending viteAppBase. If this middleware is followed by other middleware that also reads req.url, the prepended base path could cause unexpected routing. The production middleware on line 736 has the same pattern.
Consider creating a new request with the modified URL instead of mutating, or documenting that this middleware must be last in the chain.
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/plugin.ts` around lines 558 - 590, The
dev middleware inside setupMiddlewares mutates req.url by assigning
joinURL(resolvedStartConfig.viteAppBase, req.url ?? '/'), which can break
downstream middleware; instead create a new request object with the modified URL
(e.g., construct a new Request/NodeRequest using the composed URL) and pass that
to serverBuild.fetch while leaving the original req.url untouched, updating the
NodeRequest construction site (where NodeRequest({ req, res }) is created) to
use the new URL and returning sendNodeResponse(res, webRes) as before; apply the
same non-mutating change to the analogous production middleware to keep behavior
consistent.
| routerBaseUrl, | ||
| ) | ||
|
|
||
| process.env.TSS_PRERENDERING = 'true' |
There was a problem hiding this comment.
process.env.TSS_PRERENDERING is set but never cleaned up.
If the Node process continues after prerendering (e.g., in a long-lived build orchestrator), this env var persists and could affect subsequent operations.
🛡️ Proposed fix — clean up after prerendering
+ try {
const pages = await prerenderPages({ outputDir: clientOutputDir })
logger.info(`Prerendered ${pages.length} pages:`)
pages.forEach((page) => {
logger.info(`- ${page}`)
})
- } catch (error) {
- logger.error(error)
+ } finally {
+ delete process.env.TSS_PRERENDERING
}Note: this also relates to the error-swallowing issue — the catch should rethrow.
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/prerender.ts` at line 50, The code
sets process.env.TSS_PRERENDERING = 'true' but never resets it and also swallows
errors; modify the prerender flow (the function that sets
process.env.TSS_PRERENDERING in
packages/start-plugin-core/src/rsbuild/prerender.ts) to ensure the env var is
restored (delete or reset to previous value) in a finally block after
prerendering completes, and change the catch block to rethrow the caught error
so failures are propagated.
| try { | ||
| const pages = await prerenderPages({ outputDir: clientOutputDir }) | ||
|
|
||
| logger.info(`Prerendered ${pages.length} pages:`) | ||
| pages.forEach((page) => { | ||
| logger.info(`- ${page}`) | ||
| }) | ||
| } catch (error) { | ||
| logger.error(error) | ||
| } |
There was a problem hiding this comment.
Prerender errors are silently swallowed — build will appear to succeed even when prerendering fails.
The outer catch on lines 93–95 logs the error but does not rethrow. Even when failOnError is true inside addCrawlPageTask (line 244), the thrown error propagates up through the queue to prerenderPages, then to queue.start() (line 129), and is caught here — silently swallowed. The caller (postServerBuildRsbuild) will never know prerendering failed.
🐛 Proposed fix — rethrow after logging
} catch (error) {
logger.error(error)
+ throw error
}📝 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.
| try { | |
| const pages = await prerenderPages({ outputDir: clientOutputDir }) | |
| logger.info(`Prerendered ${pages.length} pages:`) | |
| pages.forEach((page) => { | |
| logger.info(`- ${page}`) | |
| }) | |
| } catch (error) { | |
| logger.error(error) | |
| } | |
| try { | |
| const pages = await prerenderPages({ outputDir: clientOutputDir }) | |
| logger.info(`Prerendered ${pages.length} pages:`) | |
| pages.forEach((page) => { | |
| logger.info(`- ${page}`) | |
| }) | |
| } catch (error) { | |
| logger.error(error) | |
| throw error | |
| } |
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/prerender.ts` around lines 86 - 95,
The catch in the prerendering block swallows errors from prerenderPages (and
upstream addCrawlPageTask/queue.start), so update the catch in the async block
around prerenderPages to log the error via logger.error(error) and then rethrow
the error so callers (e.g., postServerBuildRsbuild) observe the failure; keep
the existing logging but add a `throw error` after it to propagate the failure.
|
|
||
| const isSpaShell = | ||
| startConfig.spa?.prerender.outputPath === cleanPagePath | ||
|
|
There was a problem hiding this comment.
Potential TypeError on startConfig.spa?.prerender.outputPath if prerender is undefined.
Line 188 accesses .outputPath on startConfig.spa.prerender using optional chaining on spa but not on prerender. If spa exists but spa.prerender is undefined, this throws a TypeError. Same pattern as the issue in post-server-build.ts.
🛡️ Proposed fix
- const isSpaShell =
- startConfig.spa?.prerender.outputPath === cleanPagePath
+ const isSpaShell =
+ startConfig.spa?.prerender?.outputPath === cleanPagePath📝 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.
| const isSpaShell = | |
| startConfig.spa?.prerender.outputPath === cleanPagePath | |
| const isSpaShell = | |
| startConfig.spa?.prerender?.outputPath === cleanPagePath |
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/prerender.ts` around lines 186 - 189,
The code computing isSpaShell risks a TypeError because it uses optional
chaining on startConfig.spa but then accesses .prerender.outputPath without
guarding against prerender being undefined; update the check in the isSpaShell
assignment to safely access prerender (e.g., use
startConfig.spa?.prerender?.outputPath or check startConfig.spa?.prerender
before reading outputPath) so comparison with cleanPagePath is only performed
when outputPath is defined, preserving the existing isSpaShell logic and
variable names.
| async function loadModule( | ||
| compiler: StartCompiler, | ||
| loaderContext: any, | ||
| id: string, | ||
| ) { | ||
| const cleaned = cleanId(id) | ||
| const resolvedPath = | ||
| cleaned.startsWith('.') || cleaned.startsWith('/') | ||
| ? cleaned | ||
| : ((await resolveId(loaderContext, cleaned)) ?? cleaned) | ||
|
|
||
| if (resolvedPath.includes('\0')) return | ||
|
|
||
| try { | ||
| const code = await fsp.readFile(resolvedPath, 'utf-8') | ||
| compiler.ingestModule({ code, id: resolvedPath }) | ||
| } catch { | ||
| // ignore missing files | ||
| } | ||
| } |
There was a problem hiding this comment.
Relative-path short-circuit in loadModule may read from the wrong directory.
Lines 170–171: if cleaned starts with ./, it bypasses resolveId and is passed directly to fsp.readFile. This would resolve relative to process.cwd(), not relative to the importing module. If the StartCompiler ever calls loadModule with a relative (non-resolved) id, the read will fail or load the wrong file.
If loadModule is only ever called with already-resolved absolute paths, this is safe — but the startsWith('.') branch creates a silent trap for future callers.
🛡️ Suggested fix: always resolve relative paths
async function loadModule(
compiler: StartCompiler,
loaderContext: any,
id: string,
) {
const cleaned = cleanId(id)
- const resolvedPath =
- cleaned.startsWith('.') || cleaned.startsWith('/')
- ? cleaned
- : ((await resolveId(loaderContext, cleaned)) ?? cleaned)
+ const resolvedPath = cleaned.startsWith('/')
+ ? cleaned
+ : ((await resolveId(loaderContext, cleaned)) ?? cleaned)
if (resolvedPath.includes('\0')) return📝 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.
| async function loadModule( | |
| compiler: StartCompiler, | |
| loaderContext: any, | |
| id: string, | |
| ) { | |
| const cleaned = cleanId(id) | |
| const resolvedPath = | |
| cleaned.startsWith('.') || cleaned.startsWith('/') | |
| ? cleaned | |
| : ((await resolveId(loaderContext, cleaned)) ?? cleaned) | |
| if (resolvedPath.includes('\0')) return | |
| try { | |
| const code = await fsp.readFile(resolvedPath, 'utf-8') | |
| compiler.ingestModule({ code, id: resolvedPath }) | |
| } catch { | |
| // ignore missing files | |
| } | |
| } | |
| async function loadModule( | |
| compiler: StartCompiler, | |
| loaderContext: any, | |
| id: string, | |
| ) { | |
| const cleaned = cleanId(id) | |
| const resolvedPath = cleaned.startsWith('/') | |
| ? cleaned | |
| : ((await resolveId(loaderContext, cleaned)) ?? cleaned) | |
| if (resolvedPath.includes('\0')) return | |
| try { | |
| const code = await fsp.readFile(resolvedPath, 'utf-8') | |
| compiler.ingestModule({ code, id: resolvedPath }) | |
| } catch { | |
| // ignore missing files | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/start-compiler-loader.ts` around lines
163 - 182, The loadModule function currently short-circuits relative ids
(cleaned.startsWith('.') or '/') and calls fsp.readFile with cleaned, which
reads from process.cwd(); instead, always call resolveId(loaderContext, cleaned)
to get a proper module path (use ((await resolveId(loaderContext, cleaned)) ??
cleaned) only as a fallback for already-absolute paths), then skip if the
resolved result is falsy or contains '\0', and finally use the resolvedPath for
fsp.readFile and compiler.ingestModule; update the logic around cleanId,
resolveId, resolvedPath, fsp.readFile, and compiler.ingestModule accordingly so
relative imports are resolved relative to the importer rather than the CWD.
| export function createServerFnManifestRspackPlugin(opts: { | ||
| serverOutputDir: string | ||
| }) { | ||
| const tempManifestPath = path.join( | ||
| opts.serverOutputDir, | ||
| SERVER_FN_MANIFEST_TEMP_FILE, | ||
| ) | ||
|
|
||
| return { | ||
| apply(compiler: any) { | ||
| compiler.hooks.beforeRun.tapPromise( | ||
| 'tanstack-start:server-fn-manifest', | ||
| async () => { | ||
| resetServerFnCompilerState() | ||
| await fsp.rm(tempManifestPath, { force: true }) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
beforeRun hook won't fire for watch mode — state may leak across rebuilds.
compiler.hooks.beforeRun only fires for non-watch builds. In watch mode (e.g., rspack --watch or rsbuild dev), the watchRun hook fires instead. If this plugin is ever used in dev/watch mode, resetServerFnCompilerState() and the temp manifest cleanup won't run, potentially causing stale server function data to persist across rebuilds.
🛡️ Proposed fix — also tap into watchRun
compiler.hooks.beforeRun.tapPromise(
'tanstack-start:server-fn-manifest',
async () => {
resetServerFnCompilerState()
await fsp.rm(tempManifestPath, { force: true })
},
)
+ compiler.hooks.watchRun.tapPromise(
+ 'tanstack-start:server-fn-manifest',
+ async () => {
+ resetServerFnCompilerState()
+ await fsp.rm(tempManifestPath, { force: true })
+ },
+ )🤖 Prompt for AI Agents
In `@packages/start-plugin-core/src/rsbuild/start-compiler-plugin.ts` around lines
254 - 270, The plugin currently only hooks compiler.hooks.beforeRun so
resetServerFnCompilerState() and the tempManifestPath removal (fsp.rm) won’t run
in watch mode; update createServerFnManifestRspackPlugin to also tapPromise into
compiler.hooks.watchRun (using the same callback) so both
resetServerFnCompilerState() and await fsp.rm(tempManifestPath, { force: true })
run for watch builds as well, referencing the existing beforeRun handler logic
to avoid duplicating behavior.
Use a Response type guard-compatible response variable while keeping stricter runtime checks so start-client-core type and eslint checks pass in CI. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/start-client-core/src/client-rpc/serverFnFetcher.ts`:
- Around line 67-81: The parseRedirectFallback function uses an unsafe "as any"
cast before calling redirect; replace that with a proper cast to the redirect
input type (e.g., RedirectOptions) so TypeScript can check fields while still
allowing optional properties to be omitted—specifically change the call in
parseRedirectFallback from redirect(payload as any) to redirect(payload as
RedirectOptions) (or the exact exported type used by redirect), keeping the
existing existence checks and not adding extra validation.
| function parseRedirectFallback(payload: unknown) { | ||
| if (!payload || typeof payload !== 'object') { | ||
| return undefined | ||
| } | ||
| if (!('isSerializedRedirect' in payload)) { | ||
| return undefined | ||
| } | ||
| if ( | ||
| (payload as { isSerializedRedirect?: boolean }).isSerializedRedirect !== | ||
| true | ||
| ) { | ||
| return undefined | ||
| } | ||
| return redirect(payload as any) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the redirect function definition in the router-core package
fd -e ts -e tsx 'router' --type f | xargs grep -l "export.*function redirect" | head -20Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Find the redirect function in the codebase
rg "export\s+(function|const)\s+redirect" -t ts -t tsx -A 5Repository: TanStack/router
Length of output: 86
🏁 Script executed:
# Check the imports in the serverFnFetcher file to understand where redirect comes from
fd -e ts -e tsx 'serverFnFetcher' --type f -exec cat {} \;Repository: TanStack/router
Length of output: 246
🏁 Script executed:
# First, find the serverFnFetcher.ts file
fd 'serverFnFetcher' -t fRepository: TanStack/router
Length of output: 119
🏁 Script executed:
# Search for redirect function export using rg with correct flags
rg "export\s+(function|const)\s+redirect" --type ts -A 5Repository: TanStack/router
Length of output: 518
🏁 Script executed:
# Alternative: look for redirect in package structure
find . -name "*.ts" -type f -exec grep -l "export.*function redirect" {} \; 2>/dev/null | head -10Repository: TanStack/router
Length of output: 182
🏁 Script executed:
# Read the redirect.ts file to understand the full function signature
cat -n packages/router-core/src/redirect.tsRepository: TanStack/router
Length of output: 8122
🏁 Script executed:
# Read the serverFnFetcher.ts file to understand context
cat -n packages/start-client-core/src/client-rpc/serverFnFetcher.tsRepository: TanStack/router
Length of output: 16063
Improve type safety of the as any cast — no field validation needed.
The as any cast does bypass type checking. However, all fields in RedirectOptions are optional, and the redirect() function handles missing fields gracefully (e.g., setting a default statusCode, checking if href exists before using it). The concern about malformed redirects is unfounded.
For better type safety without overvalidating, cast to the proper type instead:
Better approach
function parseRedirectFallback(payload: unknown) {
if (!payload || typeof payload !== 'object') {
return undefined
}
if (!('isSerializedRedirect' in payload)) {
return undefined
}
if (
(payload as { isSerializedRedirect?: boolean }).isSerializedRedirect !==
true
) {
return undefined
}
- return redirect(payload as any)
+ return redirect(payload as Parameters<typeof redirect>[0])
}🤖 Prompt for AI Agents
In `@packages/start-client-core/src/client-rpc/serverFnFetcher.ts` around lines 67
- 81, The parseRedirectFallback function uses an unsafe "as any" cast before
calling redirect; replace that with a proper cast to the redirect input type
(e.g., RedirectOptions) so TypeScript can check fields while still allowing
optional properties to be omitted—specifically change the call in
parseRedirectFallback from redirect(payload as any) to redirect(payload as
RedirectOptions) (or the exact exported type used by redirect), keeping the
existing existence checks and not adding extra validation.
Remove unnecessary optional chaining in the response-like header guard so start-server-core eslint passes in CI. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
Test plan
Made with Cursor
Summary by CodeRabbit
New Features
Improvements
Chores