Skip to content

Conversation

@schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Sep 26, 2025

this first pass handles the createMiddleware imported from `@tanstack/react-start``

second pass handles

startInstance.createMiddleware()

Summary by CodeRabbit

  • New Features

    • Added first-class support for createMiddleware alongside server functions.
    • Introduced environment-aware lookup to optimize scanning on client vs. server.
    • Made lookup behavior configurable so projects can tailor which kinds are processed.
  • Tests

    • Added comprehensive middleware tests and snapshots for client and server.
    • Updated test configurations to the new lookup model; removed legacy createStart snapshots.
  • Examples

    • Added auth middleware factory examples and server/client middleware snapshots.

this first pass handles the createMiddleware imported from `@tanstack/react-start``

second pass handles

```
startInstance.createMiddleware()
```
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Walkthrough

Adds a configurable LookupKind model to ServerFnCompiler, switches candidate detection to per-kind identifiers, and makes the plugin build lookup kinds/configurations per environment. Start compiler adds createMiddleware handling. Tests and snapshots updated: new createMiddleware start-compiler tests added; legacy createMiddleware and some createStart snapshots removed or adjusted.

Changes

Cohort / File(s) Summary
ServerFnCompiler lookup kinds refactor
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts
Exported LookupKind; added LookupSetup; replaced static valid kinds with options.lookupKinds stored as this.validLookupKinds: Set<LookupKind>; updated isCandidateCallExpression signature and all internal checks to use the instance set; wired per-kind candidate identifiers.
Plugin env-driven configuration
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts
Added LookupKindsPerEnv and getLookupConfigurationsForEnv; plugin now constructs lookupKinds and lookupConfigurations per env/framework and passes them to ServerFnCompiler; adjusted include filters for scanning.
Start compiler: createMiddleware support
packages/start-plugin-core/src/start-compiler-plugin/compilers.ts, packages/start-plugin-core/src/start-compiler-plugin/constants.ts
Registered handleCreateMiddleware and added 'createMiddleware' to transformFuncs; added identifier entry for createMiddleware in the compile/start identifiers map.
New start-compiler-plugin tests & snapshots
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/createMiddleware.test.ts, packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/*, packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts, packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts
Added test harness enumerating test-files per env, comparing compiled output to snapshots; added multiple client/server snapshots demonstrating createMiddleware usage and a middleware factory example.
Tests: plugin-level middleware-only config
packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts
Test configuration switched to lookupKinds: new Set(['Middleware']); removed createServerFn lookup configuration entry.
Removed legacy createMiddleware snapshots
packages/start-plugin-core/tests/createMiddleware/snapshots/client/*
Deleted previous client snapshots that exported middleware via destructured/renamed/star imports.
ServerFn tests adjusted / snapshot removals
packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts, packages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsx, packages/start-plugin-core/tests/createServerFn/snapshots/server/createStart.tsx, packages/start-plugin-core/tests/createServerFn/test-files/createStart.tsx, packages/start-plugin-core/tests/createServerFn/snapshots/client/factory.tsx
ServerFn tests now pass lookupKinds: new Set(['ServerFn']); removed createStart-based snapshots and test-file content; retained/updated factory snapshot showing middleware chaining.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev
  participant Plugin as StartPlugin
  participant Compiler as ServerFnCompiler
  participant Scanner as AST Scanner

  Dev->>Plugin: initialize(env, framework)
  Plugin->>Plugin: lookupKinds = LookupKindsPerEnv[env]
  Plugin->>Plugin: lookupConfigurations = getLookupConfigurationsForEnv(env, framework)
  Plugin->>Compiler: new ServerFnCompiler({ lookupKinds, lookupConfigurations })
  Compiler->>Scanner: scan project files
  Scanner-->>Compiler: call expressions
  Compiler->>Compiler: isCandidateCallExpression(expr, this.validLookupKinds)
  alt matches ServerFn
    Compiler->>Compiler: check identifier 'handler'
  else matches Middleware
    Compiler->>Compiler: check identifiers 'server'|'client'|'createMiddlewares'
  end
  Compiler-->>Plugin: candidates collected
Loading
sequenceDiagram
  autonumber
  participant Source as Source Code
  participant StartC as Start Compiler
  participant Handlers as Handlers Map
  participant Mid as handleCreateMiddleware

  Source->>StartC: compileStartOutputFactory('react')
  StartC->>Handlers: register handlers (server, client, isomorphic, createMiddleware)
  Source->>StartC: encounter createMiddleware(...)
  StartC->>Mid: handleCreateMiddleware(call)
  Mid-->>StartC: transformed output
  StartC-->>Source: compiled result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

A hop, a sniff, new kinds to find,
I scan the AST with a curious mind.
Middleware and ServerFn take their place,
Per-env trails guide each call's trace.
Thump! Tests updated — I nourish the base. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and concisely describes the primary change in this pull request—adding support to compile createMiddleware using the start-compiler-plugin—and directly reflects the two-pass compilation approach outlined in the objectives. It avoids unnecessary detail or jargon and clearly signals the core fix being applied.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch createMiddlware-2pass

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0886efb and 3c1ece7.

📒 Files selected for processing (2)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts

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

@nx-cloud
Copy link

nx-cloud bot commented Sep 26, 2025

View your CI Pipeline Execution ↗ for commit 3c1ece7

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 1m 13s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 2s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-26 03:25:05 UTC

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (2)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (2)

34-42: Consider extracting candidate identifier sets as constants.

While the LookupSetup configuration is well-structured, the hardcoded string values could be extracted as constants to improve maintainability and prevent typos.

Consider this refactor:

+const SERVER_FN_CANDIDATES = ['handler'] as const
+const MIDDLEWARE_CANDIDATES = ['server', 'client', 'createMiddlewares'] as const
+
 const LookupSetup: Record<
   LookupKind,
   { candidateCallIdentifier: Set<string> }
 > = {
-  ServerFn: { candidateCallIdentifier: new Set(['handler']) },
+  ServerFn: { candidateCallIdentifier: new Set(SERVER_FN_CANDIDATES) },
   Middleware: {
-    candidateCallIdentifier: new Set(['server', 'client', 'createMiddlewares']),
+    candidateCallIdentifier: new Set(MIDDLEWARE_CANDIDATES),
   },
 }

425-443: Consider simplifying the nested conditional logic.

The member expression resolution logic could be refactored to reduce duplication and improve readability.

Consider extracting the common pattern:

+  private async checkCandidateKind(
+    kind: LookupKind,
+    prop: string,
+    callee: t.MemberExpression,
+    fileId: string,
+    visited: Set<string>,
+  ): Promise<Kind | null> {
+    if (!this.validLookupKinds.has(kind) || !LookupSetup[kind].candidateCallIdentifier.has(prop)) {
+      return null;
+    }
+    const base = await this.resolveExprKind(callee.object, fileId, visited);
+    const validBases = kind === 'ServerFn' 
+      ? ['Root', 'Builder'] 
+      : ['Root', 'Builder', 'Middleware'];
+    return validBases.includes(base as string) ? kind : 'None';
+  }

   if (t.isMemberExpression(callee) && t.isIdentifier(callee.property)) {
     const prop = callee.property.name

-    if (
-      this.validLookupKinds.has('ServerFn') &&
-      LookupSetup['ServerFn'].candidateCallIdentifier.has(prop)
-    ) {
-      const base = await this.resolveExprKind(callee.object, fileId, visited)
-      if (base === 'Root' || base === 'Builder') {
-        return 'ServerFn'
-      }
-      return 'None'
-    } else if (
-      this.validLookupKinds.has('Middleware') &&
-      LookupSetup['Middleware'].candidateCallIdentifier.has(prop)
-    ) {
-      const base = await this.resolveExprKind(callee.object, fileId, visited)
-      if (base === 'Root' || base === 'Builder' || base === 'Middleware') {
-        return 'Middleware'
-      }
-      return 'None'
-    }
+    const serverFnResult = await this.checkCandidateKind('ServerFn', prop, callee, fileId, visited);
+    if (serverFnResult !== null) return serverFnResult;
+    
+    const middlewareResult = await this.checkCandidateKind('Middleware', prop, callee, fileId, visited);
+    if (middlewareResult !== null) return middlewareResult;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7771c00 and 0886efb.

📒 Files selected for processing (22)
  • packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (7 hunks)
  • packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (4 hunks)
  • packages/start-plugin-core/src/start-compiler-plugin/compilers.ts (2 hunks)
  • packages/start-plugin-core/src/start-compiler-plugin/constants.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/createMiddleware.test.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/create-function-middleware.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructured.tsx (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructuredRename.tsx (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareStarImport.tsx (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareValidator.tsx (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts (1 hunks)
  • packages/start-plugin-core/tests/createMiddleware/snapshots/client/createMiddlewareDestructured.tsx (0 hunks)
  • packages/start-plugin-core/tests/createMiddleware/snapshots/client/createMiddlewareDestructuredRename.tsx (0 hunks)
  • packages/start-plugin-core/tests/createMiddleware/snapshots/client/createMiddlewareStarImport.tsx (0 hunks)
  • packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts (1 hunks)
  • packages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsx (0 hunks)
  • packages/start-plugin-core/tests/createServerFn/snapshots/client/factory.tsx (1 hunks)
  • packages/start-plugin-core/tests/createServerFn/snapshots/server/createStart.tsx (0 hunks)
  • packages/start-plugin-core/tests/createServerFn/test-files/createStart.tsx (0 hunks)
💤 Files with no reviewable changes (6)
  • packages/start-plugin-core/tests/createServerFn/snapshots/client/createStart.tsx
  • packages/start-plugin-core/tests/createMiddleware/snapshots/client/createMiddlewareDestructuredRename.tsx
  • packages/start-plugin-core/tests/createServerFn/snapshots/server/createStart.tsx
  • packages/start-plugin-core/tests/createMiddleware/snapshots/client/createMiddlewareDestructured.tsx
  • packages/start-plugin-core/tests/createServerFn/test-files/createStart.tsx
  • packages/start-plugin-core/tests/createMiddleware/snapshots/client/createMiddlewareStarImport.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • packages/start-plugin-core/src/start-compiler-plugin/compilers.ts
  • packages/start-plugin-core/src/start-compiler-plugin/constants.ts
  • packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
  • packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareValidator.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/createMiddleware.test.ts
  • packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts
  • packages/start-plugin-core/tests/createServerFn/snapshots/client/factory.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructured.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareStarImport.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructuredRename.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/create-function-middleware.ts
packages/{*-start,start-*}/**

📄 CodeRabbit inference engine (AGENTS.md)

Name and place Start framework packages under packages/-start/ or packages/start-/

Files:

  • packages/start-plugin-core/src/start-compiler-plugin/compilers.ts
  • packages/start-plugin-core/src/start-compiler-plugin/constants.ts
  • packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
  • packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareValidator.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/createMiddleware.test.ts
  • packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts
  • packages/start-plugin-core/tests/createServerFn/snapshots/client/factory.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructured.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareStarImport.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructuredRename.tsx
  • packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/create-function-middleware.ts
🧬 Code graph analysis (9)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (2)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (2)
  • LookupKind (32-32)
  • LookupConfig (44-47)
packages/start-plugin-core/src/start-compiler-plugin/compilers.ts (1)
  • CompileStartFrameworkOptions (18-18)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/createMiddleware.test.ts (1)
packages/start-plugin-core/src/start-compiler-plugin/compilers.ts (1)
  • compileStartOutputFactory (22-153)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (1)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (2)
  • handler (63-67)
  • handler (91-153)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts (2)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts (1)
  • createAuthMiddleware (11-39)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts (1)
  • createAuthMiddleware (12-38)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts (2)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts (1)
  • createAuthMiddleware (11-15)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts (1)
  • createAuthMiddleware (11-39)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructured.tsx (2)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/createMiddlewareDestructured.tsx (4)
  • withUseServer (3-9)
  • withoutUseServer (10-16)
  • withVariable (17-19)
  • withZodValidator (30-36)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/createMiddlewareDestructured.tsx (4)
  • withUseServer (4-12)
  • withoutUseServer (14-22)
  • withVariable (24-26)
  • withZodValidator (45-51)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts (2)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts (1)
  • createAuthMiddleware (11-15)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts (1)
  • createAuthMiddleware (12-38)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareStarImport.tsx (2)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/createMiddlewareStarImport.tsx (4)
  • withUseServer (3-11)
  • withoutUseServer (12-18)
  • withVariable (19-21)
  • withZodValidator (32-38)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/createMiddlewareStarImport.tsx (4)
  • withUseServer (4-13)
  • withoutUseServer (15-23)
  • withVariable (25-27)
  • withZodValidator (46-52)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructuredRename.tsx (2)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/createMiddlewareDestructuredRename.tsx (4)
  • withUseServer (3-9)
  • withoutUseServer (10-16)
  • withVariable (17-19)
  • withZodValidator (30-36)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/createMiddlewareDestructuredRename.tsx (4)
  • withUseServer (4-12)
  • withoutUseServer (14-22)
  • withVariable (24-26)
  • withZodValidator (45-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test
🔇 Additional comments (14)
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareValidator.tsx (1)

2-2: Validator import mirrors compiler output

Good to see the snapshot now capturing the zod import that the compiler emits for validator-enabled middleware.

packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts (1)

20-27: Targeting only ServerFn lookups is correct

Explicitly scoping the compiler to the ServerFn lookup kind keeps this fixture focused on the server-fn surface while satisfying the new configuration contract. 👍

packages/start-plugin-core/tests/createServerFn/snapshots/client/factory.tsx (2)

4-22: Middleware snapshot captures the expected context layering

The generated server middleware wrappers now surface the context augmentation exactly as the new two-pass compiler should. This matches the intended Start API usage and keeps the snapshot aligned with runtime behavior.


24-35: Chained server function factories look good

The snapshot clearly shows the composed middleware chain feeding into the handlers, making it easy to confirm the second pass builds on the first without regressions. All good here.

packages/start-plugin-core/src/start-compiler-plugin/compilers.ts (2)

9-9: LGTM! Clean integration of middleware handling.

The import of handleCreateMiddleware from the create-server-fn-plugin module follows the existing pattern and properly integrates with the compiler infrastructure.


42-46: LGTM! Middleware identifier properly configured.

The createMiddleware entry is correctly added to the identifiers map with the appropriate handler and paths initialization, following the same pattern as other transform functions.

packages/start-plugin-core/src/start-compiler-plugin/constants.ts (1)

1-6: LGTM! Consistent with compiler implementation.

The addition of 'createMiddleware' to the transformFuncs array properly extends the set of available transformations and aligns with the createMiddleware handler implementation in compilers.ts.

packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (5)

32-32: LGTM! Well-structured type definition.

The exported LookupKind type clearly differentiates between 'ServerFn' and 'Middleware', providing strong type safety for the lookup system.


59-70: LGTM! Clean initialization of lookup kinds.

The addition of validLookupKinds as a private field and its initialization from the options is properly implemented, providing a clean way to configure which lookup kinds are active.


222-224: LGTM! Type-safe validation of lookup kinds.

The use of Set.has() with proper type casting to LookupKind ensures type safety while efficiently checking for valid lookup kinds.


253-259: LGTM! Proper dispatch to middleware handler.

The conditional dispatch correctly routes ServerFn to handleCreateServerFn and Middleware to handleCreateMiddleware, with appropriate environment context passed to both handlers.


501-518: LGTM! Efficient candidate detection with configurable lookup kinds.

The refactored isCandidateCallExpression function properly iterates through the configured lookup kinds and checks against their respective candidate identifiers.

packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts (1)

20-30: LGTM! Test configuration properly validates middleware-only compilation.

The test correctly configures the ServerFnCompiler with only the 'Middleware' lookup kind and appropriate lookup configurations for both createMiddleware and createStart from '@tanstack/react-start'.

packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/create-function-middleware.ts (1)

2-2: Import is part of the test fixture and mirrored in both server and client snapshots; no action needed.

Likely an incorrect or invalid review comment.

Comment on lines +86 to +88
// TODO apply this plugin with a different filter per environment so that .createMiddleware() calls are not scanned in server env
// only scan files that mention `.handler(` | `.createMiddleware()`
include: [/\.handler\(/, /.createMiddleware\(\)/],
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix the createMiddleware filter before shipping

The new /.createMiddleware\(\)/ filter only matches a literal createMiddleware() call with empty parentheses. Real code always has arguments (createMiddleware({...}), start.createMiddleware(...)), so the transform hook never receives those modules and the middleware rewrite never runs. Please broaden the regex so any createMiddleware( call is caught.

-            include: [/\.handler\(/, /.createMiddleware\(\)/],
+            include: [/\.handler\(/, /createMiddleware\(/],
📝 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
// TODO apply this plugin with a different filter per environment so that .createMiddleware() calls are not scanned in server env
// only scan files that mention `.handler(` | `.createMiddleware()`
include: [/\.handler\(/, /.createMiddleware\(\)/],
// TODO apply this plugin with a different filter per environment so that .createMiddleware() calls are not scanned in server env
// only scan files that mention `.handler(` | `.createMiddleware()`
include: [/\.handler\(/, /createMiddleware\(/],
🤖 Prompt for AI Agents
In packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts around lines
86 to 88, the include filter /\.createMiddleware\(\)/ only matches a literal
empty-argument call and misses real usages like createMiddleware({...}) or
start.createMiddleware(...); change that entry to a broader regex (e.g.
/createMiddleware\(/) so any call site containing "createMiddleware(" is matched
and the transform hook runs for those modules.

Comment on lines +12 to +36
describe('createMiddleware compiles correctly', async () => {
const filenames = await getFilenames()

describe.each(filenames)('should handle "%s"', async (filename) => {
const file = await readFile(
path.resolve(import.meta.dirname, `./test-files/${filename}`),
)
const code = file.toString()

test.each(['client', 'server'] as const)(
`should compile for ${filename} %s`,
async (env) => {
const compiledResult = compileStartOutput({
env,
code,
filename,
dce: false,
})

await expect(compiledResult.code).toMatchFileSnapshot(
`./snapshots/${env}/${filename}`,
)
},
)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Make describe blocks synchronous

describe/describe.each callbacks must register tests synchronously. Marking them async makes Vitest throw (The callback function of describe(...) should not return a value), so none of the contained tests will run.

Restructure to resolve the filenames up front (top-level await is fine in these ESM test files) and keep the describe callbacks synchronous, e.g.:

 import { describe, expect, test } from 'vitest'
 import { compileStartOutputFactory } from '../../src/start-compiler-plugin/compilers'

 const compileStartOutput = compileStartOutputFactory('react')

 async function getFilenames() {
   return await readdir(path.resolve(import.meta.dirname, './test-files'))
 }

-describe('createMiddleware compiles correctly', async () => {
-  const filenames = await getFilenames()
-
-  describe.each(filenames)('should handle "%s"', async (filename) => {
-    const file = await readFile(
-      path.resolve(import.meta.dirname, `./test-files/${filename}`),
-    )
-    const code = file.toString()
-
-    test.each(['client', 'server'] as const)(
-      `should compile for ${filename} %s`,
-      async (env) => {
-        const compiledResult = compileStartOutput({
-          env,
-          code,
-          filename,
-          dce: false,
-        })
-
-        await expect(compiledResult.code).toMatchFileSnapshot(
-          `./snapshots/${env}/${filename}`,
-        )
-      },
-    )
-  })
-})
+const filenames = await getFilenames()
+
+describe('createMiddleware compiles correctly', () => {
+  describe.each(filenames)('should handle "%s"', (filename) => {
+    test.each(['client', 'server'] as const)(
+      `should compile for ${filename} %s`,
+      async (env) => {
+        const file = await readFile(
+          path.resolve(import.meta.dirname, `./test-files/${filename}`),
+        )
+        const code = file.toString()
+
+        const compiledResult = compileStartOutput({
+          env,
+          code,
+          filename,
+          dce: false,
+        })
+
+        await expect(compiledResult.code).toMatchFileSnapshot(
+          `./snapshots/${env}/${filename}`,
+        )
+      },
+    )
+  })
+})
📝 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
describe('createMiddleware compiles correctly', async () => {
const filenames = await getFilenames()
describe.each(filenames)('should handle "%s"', async (filename) => {
const file = await readFile(
path.resolve(import.meta.dirname, `./test-files/${filename}`),
)
const code = file.toString()
test.each(['client', 'server'] as const)(
`should compile for ${filename} %s`,
async (env) => {
const compiledResult = compileStartOutput({
env,
code,
filename,
dce: false,
})
await expect(compiledResult.code).toMatchFileSnapshot(
`./snapshots/${env}/${filename}`,
)
},
)
})
import { describe, expect, test } from 'vitest'
import { compileStartOutputFactory } from '../../src/start-compiler-plugin/compilers'
const compileStartOutput = compileStartOutputFactory('react')
async function getFilenames() {
return await readdir(path.resolve(import.meta.dirname, './test-files'))
}
const filenames = await getFilenames()
describe('createMiddleware compiles correctly', () => {
describe.each(filenames)('should handle "%s"', (filename) => {
test.each(['client', 'server'] as const)(
`should compile for ${filename} %s`,
async (env) => {
const file = await readFile(
path.resolve(import.meta.dirname, `./test-files/${filename}`),
)
const code = file.toString()
const compiledResult = compileStartOutput({
env,
code,
filename,
dce: false,
})
await expect(compiledResult.code).toMatchFileSnapshot(
`./snapshots/${env}/${filename}`,
)
},
)
})
})
🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/createMiddleware.test.ts
around lines 12 to 36, the describe and describe.each callbacks are marked async
which causes Vitest to error because describe callbacks must register tests
synchronously; remove the async keywords and resolve filenames and file reads
before the describe blocks (top-level await is fine in these ESM tests) so the
describe/describe.each callbacks run synchronously and only the inner test
functions remain async where needed.

Comment on lines +12 to +20
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
function zodValidator<TSchema extends z.ZodSchema, TResult>(schema: TSchema, fn: (input: z.output<TSchema>) => TResult) {
return async (input: unknown) => {
return fn(schema.parse(input));
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Restore missing axios/PostType imports

abstractedFunction still references axios and PostType, but the client snapshot no longer brings either symbol into scope. TypeScript fails immediately (Cannot find name 'axios' / 'PostType') even though these helpers end up unused after the client rewrite. Please reintroduce the import and type alias (mirroring the source fixture) so the emitted snapshot stays type-checkable.

+import axios from 'axios';
+
+type PostType = {
+  userId: number;
+  id: number;
+  title: string;
+  body: string;
+};
📝 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
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
function zodValidator<TSchema extends z.ZodSchema, TResult>(schema: TSchema, fn: (input: z.output<TSchema>) => TResult) {
return async (input: unknown) => {
return fn(schema.parse(input));
};
import axios from 'axios';
type PostType = {
userId: number;
id: number;
title: string;
body: string;
};
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
function zodValidator<TSchema extends z.ZodSchema, TResult>(schema: TSchema, fn: (input: z.output<TSchema>) => TResult) {
return async (input: unknown) => {
return fn(schema.parse(input));
};
🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructured.tsx
around lines 12 to 20, the snapshot references axios and PostType but the
imports/type alias were removed; reintroduce an import for axios and re-declare
the PostType type alias to match the original fixture (so TypeScript can resolve
axios and PostType), placing the import at the top of the file and the type
alias before abstractedFunction so the snapshot is type-checkable.

Comment on lines +12 to +16
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Drop unused abstractedFunction that leaks server dependencies.

After the transform, nothing references abstractedFunction. Keeping it around forces the generated client bundle to reference axios/PostType without importing them, which fails compilation. Please have the plugin prune this function so the emitted client code stays self-contained.

Apply this diff to remove the dead function:

-async function abstractedFunction() {
-  console.info('Fetching posts...');
-  await new Promise(r => setTimeout(r, 500));
-  return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
-}
📝 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
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareDestructuredRename.tsx
around lines 12 to 16, an unused function named abstractedFunction remains after
transform and leaks server-only dependencies (axios, PostType) into the client
bundle; remove/prune this dead function from the transformed output so the
emitted client code no longer references those server dependencies and the
bundle compiles cleanly.

Comment on lines +12 to +16
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Strip unused server helper from client output.

Same issue here—the client snapshot still includes abstractedFunction, which pulls in axios/PostType symbols that are no longer imported. This leaves the compiled client code in an uncompilable state. The transform should omit this helper when emitting the client version.

Apply this diff to excise the unused helper:

-async function abstractedFunction() {
-  console.info('Fetching posts...');
-  await new Promise(r => setTimeout(r, 500));
-  return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
-}
📝 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
async function abstractedFunction() {
console.info('Fetching posts...');
await new Promise(r => setTimeout(r, 500));
return axios.get<Array<PostType>>('https://jsonplaceholder.typicode.com/posts').then(r => r.data.slice(0, 10));
}
🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/createMiddlewareStarImport.tsx
around lines 12-16, the snapshot still contains the server-only helper
`abstractedFunction` (and its axios/PostType dependencies) even though it's
unused in client output; modify the transform that emits client code to detect
and strip unused server helpers before serializing the snapshot—specifically, if
a helper function (like `abstractedFunction`) and its imports are not referenced
from any client-exported symbols, omit emitting that function and its related
imports so the compiled client artifact does not reference axios/PostType and
remains compilable.

Comment on lines 1 to 2
import { createMiddleware } from "@tanstack/react-start";
import { getCookie } from "@tanstack/react-start/server";
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Remove server-only import from client snapshot.

getCookie is a server-only helper, yet the client compilation output still imports it. Any client bundler will attempt to resolve @tanstack/react-start/server, which breaks the client build. The transform should strip this import when emitting the client snapshot.

Apply this diff to drop the import:

-import { getCookie } from "@tanstack/react-start/server";
📝 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 { createMiddleware } from "@tanstack/react-start";
import { getCookie } from "@tanstack/react-start/server";
// packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts
-import { getCookie } from "@tanstack/react-start/server";
🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/client/factory.ts
lines 1-2, the snapshot erroneously imports the server-only helper getCookie
from "@tanstack/react-start/server"; update the client-emitting transform so it
strips any imports from "@tanstack/react-start/server" when generating client
snapshots. Concretely: detect imports whose source equals
"@tanstack/react-start/server" (or server-only exports), remove those import
specifiers or the entire import statement for client targets, and update the
snapshot emitter/tests to expect the cleaned import list so client bundles no
longer reference server-only modules.

Comment on lines 15 to 22
}).server<AuthContext>(({
next
}) => {
const token = getCookie("session");
if (!token) {
if (!opts.allowUnauthenticated) {
throw new Error("Unauthorized");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Await getCookie in server middleware

getCookie from @tanstack/react-start/server resolves asynchronously (Promise<string | undefined>). Without awaiting it, token is a Promise, so if (!token) never trips and every unauthenticated request falls through as “authenticated” with a random session. Please mark the handler async and await the cookie lookup, then refresh the snapshot.

-}).server<AuthContext>(({
-  next
-}) => {
-  const token = getCookie("session");
+}).server<AuthContext>(async ({
+  next
+}) => {
+  const token = await getCookie("session");
📝 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
}).server<AuthContext>(({
next
}) => {
const token = getCookie("session");
if (!token) {
if (!opts.allowUnauthenticated) {
throw new Error("Unauthorized");
}
}).server<AuthContext>(async ({
next
}) => {
const token = await getCookie("session");
if (!token) {
if (!opts.allowUnauthenticated) {
throw new Error("Unauthorized");
}
🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/snapshots/server/factory.ts
around lines 15-22, the server middleware calls getCookie which returns a
Promise but is not awaited, causing token to be a Promise and bypassing the
unauthenticated check; make the middleware handler async and await getCookie
(const token = await getCookie("session")), keep the rest of the logic
unchanged, and then refresh the snapshot to reflect the async change.

Comment on lines +15 to +23
createMiddleware({ type: 'function' }).server<AuthContext>(({ next }) => {
const token = getCookie('session')
if (!token) {
if (!opts.allowUnauthenticated) {
throw new Error('Unauthorized')
}

return next({
context: {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Await getCookie to enforce the auth guard

getCookie returns Promise<string | undefined>. Right now token is that Promise, so if (!token) never fires and unauthenticated clients always reach the success branch with a fake session. Make the middleware async and await the cookie before branching.

-) =>
-  createMiddleware({ type: 'function' }).server<AuthContext>(({ next }) => {
-    const token = getCookie('session')
+) =>
+  createMiddleware({ type: 'function' }).server<AuthContext>(async ({ next }) => {
+    const token = await getCookie('session')

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
packages/start-plugin-core/tests/createMiddleware-start-compiler-plugin/test-files/factory.ts
around lines 15 to 23, the handler calls getCookie which returns a Promise but
does not await it so the auth check is ineffective; make the middleware handler
async, await getCookie('session') into a token variable, then use that resolved
token in the if (!token) branch (and adjust any generic/type annotation if
needed for an async function) so unauthenticated requests are correctly rejected
when opts.allowUnauthenticated is false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants