fix: compile factories #6188
fix: compile factories #6188
Conversation
WalkthroughAdds middleware-factory support and environment-aware pre-detection for code kinds in the ServerFn compiler and plugin, updates the e2e app with a middleware-factory route and test, and expands test fixtures/snapshots for isomorphic, middleware, and env-only factories. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit df1a55f
☁️ 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: 0
🧹 Nitpick comments (1)
packages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.ts (1)
1-20: Snapshot correctly demonstrates server code stripping for client builds.This snapshot correctly shows the expected client-side output where all
.server()calls and their implementations are stripped, leaving only the middleware factory structure. This demonstrates the PR's fix: preventing server-only imports (likerateLimit,getUser) from being processed in client builds.However, based on coding guidelines for snapshots, consider adding a clearer header comment at the top describing the transformation being tested, for example:
// Expected client output: Middleware factories with server-only code stripped // Input: packages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.tsAlso note that the inline comments (lines 3, 10, 17) reference
.server()calls, but this snapshot intentionally omits them—consider updating those comments to reflect that this is the client-side output.Based on learnings, snapshot files should have header comments describing the input scenario being tested.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
e2e/react-start/server-functions/src/routeTree.gen.tse2e/react-start/server-functions/src/routes/middleware/index.tsxe2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsxe2e/react-start/server-functions/tests/server-functions.spec.tspackages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/start-plugin-core/src/create-server-fn-plugin/plugin.tspackages/start-plugin-core/tests/compiler.test.tspackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.tspackages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.tspackages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsxpackages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/compiler.test.tse2e/react-start/server-functions/tests/server-functions.spec.tspackages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.tspackages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsxe2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsxpackages/start-plugin-core/src/create-server-fn-plugin/plugin.tspackages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.tse2e/react-start/server-functions/src/routeTree.gen.tspackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnInline.tsxe2e/react-start/server-functions/src/routes/middleware/index.tsxpackages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/compiler.test.tse2e/react-start/server-functions/tests/server-functions.spec.tspackages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.tspackages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsxe2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsxpackages/start-plugin-core/src/create-server-fn-plugin/plugin.tspackages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.tse2e/react-start/server-functions/src/routeTree.gen.tspackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnInline.tsxe2e/react-start/server-functions/src/routes/middleware/index.tsxpackages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-16T02:59:11.506Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6104
File: packages/start-plugin-core/tests/split-exports-plugin/snapshots/exports/functionExports.ts:1-1
Timestamp: 2025-12-16T02:59:11.506Z
Learning: In transformation test snapshots (e.g., split-exports plugin), comments at the top of snapshot files often describe the original input scenario being tested (e.g., "Multiple function exports") rather than the transformed output in the snapshot itself. This helps document what transformation is being validated.
Applied to files:
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsxe2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsxpackages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.ts
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
e2e/react-start/server-functions/tests/server-functions.spec.tspackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsxpackages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsxe2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsxpackages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.tspackages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsxpackages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx
📚 Learning: 2025-12-16T02:59:06.535Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6104
File: packages/start-plugin-core/tests/split-exports-plugin/snapshots/exports/functionExports.ts:1-1
Timestamp: 2025-12-16T02:59:06.535Z
Learning: When adding or updating snapshot files (especially transformation tests like split-exports-plugin), place a brief header comment at the top describing the input scenario being tested (e.g., "Input: Multiple function exports"). This documents what transformation is validated and makes snapshots self-describing. Apply this pattern to all TS snapshot files under any snapshots directory.
Applied to files:
packages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
e2e/react-start/server-functions/src/routeTree.gen.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
e2e/react-start/server-functions/src/routeTree.gen.ts
🧬 Code graph analysis (10)
packages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.ts (1)
packages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.ts (3)
createPublicRateLimitMiddleware(4-8)createAuthMiddleware(11-15)topLevelMiddleware(18-20)
packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsx (2)
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnInline.tsx (6)
getPlatformValue(2-5)getEnvironment(8-11)getDirectValue(14-16)getMultipleValues(19-26)getServerOnlyValue(29-32)getClientOnlyValue(35-41)packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsx (6)
getPlatformValue(2-5)getEnvironment(8-11)getDirectValue(14-16)getMultipleValues(19-26)getServerOnlyValue(29-35)getClientOnlyValue(38-41)
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsx (2)
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsx (5)
createPlatformFn(2-4)createServerImplFn(7-12)createClientImplFn(15-17)createNoImplFn(20-22)topLevelIsomorphicFn(25-25)packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsx (5)
createPlatformFn(4-8)createServerImplFn(11-16)createClientImplFn(19-24)createNoImplFn(27-29)topLevelIsomorphicFn(32-34)
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsx (2)
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsx (5)
createPlatformFn(2-4)createServerImplFn(7-9)createClientImplFn(12-17)createNoImplFn(20-22)topLevelIsomorphicFn(25-25)packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsx (5)
createPlatformFn(4-8)createServerImplFn(11-16)createClientImplFn(19-24)createNoImplFn(27-29)topLevelIsomorphicFn(32-34)
packages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsx (2)
packages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx (6)
createServerFactory(2-7)createClientFactory(10-14)createServerArrowFactory(17-19)createClientArrowFactory(22-26)topLevelServerFn(29-29)topLevelClientFn(30-32)packages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsx (6)
createServerFactory(4-9)createClientFactory(12-17)createServerArrowFactory(20-22)createClientArrowFactory(25-27)topLevelServerFn(30-30)topLevelClientFn(31-31)
e2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsx (4)
packages/start-server-core/src/request-response.ts (1)
getRequestHeaders(149-152)e2e/react-start/server-functions/src/routes/middleware/index.tsx (1)
Route(3-5)examples/solid/kitchen-sink/src/useMutation.tsx (1)
data(47-49)packages/router-core/src/ssr/tsrScript.ts (1)
p(16-18)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (1)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (4)
LookupKindsPerEnv(84-98)KindDetectionPatterns(75-81)LookupKind(35-40)detectKindsInCode(104-120)
packages/start-plugin-core/tests/createMiddleware/snapshots/client/middleware-factory.ts (1)
packages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.ts (3)
createPublicRateLimitMiddleware(4-19)createAuthMiddleware(22-33)topLevelMiddleware(36-41)
e2e/react-start/server-functions/src/routes/middleware/index.tsx (3)
e2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsx (1)
Route(72-74)e2e/react-start/server-functions/src/routes/middleware/server-import-middleware.tsx (1)
Route(36-38)e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx (1)
Route(23-26)
packages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsx (2)
packages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsx (6)
createServerFactory(2-6)createClientFactory(9-14)createServerArrowFactory(17-21)createClientArrowFactory(24-26)topLevelServerFn(29-31)topLevelClientFn(32-32)packages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx (6)
createServerFactory(2-7)createClientFactory(10-14)createServerArrowFactory(17-19)createClientArrowFactory(22-26)topLevelServerFn(29-29)topLevelClientFn(30-32)
⏰ 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). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (29)
e2e/react-start/server-functions/src/routes/middleware/index.tsx (1)
44-52: LGTM!The new navigation link follows the established pattern of existing middleware test links and includes an appropriate
data-testidfor e2e testing.e2e/react-start/server-functions/src/routeTree.gen.ts (1)
1-9: Auto-generated file - no issues.This file is automatically generated by TanStack Router (as noted in the header). The additions for the new
MiddlewareMiddlewareFactoryRouteare correctly registered across all route mappings.e2e/react-start/server-functions/src/routes/middleware/middleware-factory.tsx (3)
6-13: Excellent documentation.The comment clearly explains the test's purpose and the expected failure mode if the fix doesn't work, which is valuable for future maintainers.
16-55: Good coverage of factory patterns.Testing both regular function and arrow function factory variants ensures the fix handles different code patterns that developers might use.
76-126: LGTM!The component correctly handles both success and error states, includes appropriate test IDs for e2e testing, and the state typing is explicit.
e2e/react-start/server-functions/tests/server-functions.spec.ts (1)
639-666: Well-structured e2e test.The test follows the established pattern from the existing
server-import-middlewaretest and includes good documentation explaining what's being verified. The assertions correctly check both the custom header value and the prefixed headers from the factory middleware.packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (4)
12-19: Good addition for virtual module handling.The null byte prefix stripping is important for Vite/Rollup virtual modules. This ensures consistent ID handling across different module types.
21-33: Single source of truth pattern.Deriving the transform code filter from
KindDetectionPatternsandLookupKindsPerEnvensures consistency and reduces the risk of filter mismatches. This is a good refactor that centralizes the pattern definitions.
168-178: Core fix implementation looks correct.Pre-detecting kinds before parsing allows the compiler to filter out server-only code paths (like middleware factories) from the client bundle. This addresses the root cause where server-only imports inside factory functions were being analyzed in client builds.
99-100: Environment-specific filtering is correctly applied.The
transformCodeFilteris now derived per environment, ensuring that only relevant patterns trigger the transform. For the client environment, this includes theMiddlewarepattern which will catch middleware factories.packages/start-plugin-core/tests/envOnly/snapshots/server/envOnlyFactory.tsx (1)
1-32: LGTM! Server-side snapshot correctly demonstrates environment-specific transformations.The snapshot properly shows:
- Server factories return functional implementations
- Client factories throw errors indicating they cannot run on the server
- Both function declaration and arrow function patterns are covered
Based on learnings, comments describe the original input scenario being tested.
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (6)
83-98: Key fix for issue #6185: Middleware excluded from server environment.This is the core change that prevents server-only middleware imports (like
cloudflare:workers) from being processed in client builds. By only includingMiddlewarein the client environment, the compiler will skip middleware processing on the server side, allowing proper tree-shaking.
71-81: Pattern design aligns with detection strategies.The asymmetry between patterns is intentional:
ServerFnuses.handler\s*\(to match the terminal method in method chains- Other kinds use factory function name patterns for direct call detection
This correctly mirrors the
LookupSetupconfiguration whereServerFnusesmethodChaintype whileServerOnlyFn/ClientOnlyFnusedirectCalltype.
100-120: Efficient pre-scan optimization.The
detectKindsInCodefunction provides fast string-based detection before expensive AST parsing. The environment filtering ensures only relevant kinds are checked, reducing unnecessary work.
179-231: Sound two-tier candidate detection strategy.The distinction between nested and top-level detection is well-designed:
- Nested (stricter): Only known factory names to avoid false positives from invocations of already-created functions
- Top-level (more lenient): Accepts any simple call at program level, delegating to resolution for verification
This handles renamed imports correctly at the top level while preventing noise from nested calls.
424-500: Well-structured compile method with early exits.The updated
compilemethod efficiently:
- Accepts optional
detectedKindsfor pre-filtering- Intersects with
validLookupKindsfor safety- Provides multiple early exit points when no work is needed
- Uses single-pass traversal to collect candidates
The
chainCallPathsmap for storing inner chain calls is a smart optimization for method chain lookup.
913-939: Clean refactor to boolean return type.The function now returns a simple boolean, making the API cleaner. The O(1) lookup via
IdentifierToKindsand iteration over possible kinds correctly handles identifiers shared by multiple kinds.packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnFactory.tsx (1)
1-25: LGTM! Client-side snapshot correctly demonstrates isomorphic function transformations.Verified against the test-files input:
- Functions with both client/server implementations retain client impl
- Server-only implementations become empty functions
- Client-only implementations are preserved with full logic
- Symmetry with server snapshot is maintained
Based on learnings, comments describe the original input scenario being tested.
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnInline.tsx (1)
1-41: LGTM! Server-side inline snapshot correctly demonstrates transformations.The snapshot properly shows:
- Server-only implementations are preserved with full logic (including
console.logside effects)- Client-only implementations become empty functions
- Multiple inline isomorphic functions are handled correctly
- Direct invocation patterns are supported
Based on learnings, comments describe the original input scenario being tested.
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnFactory.tsx (1)
1-25: LGTM! Server-side factory snapshot correctly demonstrates transformations with proper symmetry.Verified against the client snapshot and test-files input:
createPlatformFn: Returnsserver-${platform}(vs client'sclient-${platform})createServerImplFn: Full implementation preserved (vs client's empty function)createClientImplFn: Empty function (vs client's full implementation)- Proper symmetry maintained between server and client snapshots
Based on learnings, comments describe the original input scenario being tested.
packages/start-plugin-core/tests/compiler.test.ts (1)
1-393: Excellent comprehensive test coverage for environment-aware kind detection!The test suite thoroughly validates:
- Pattern detection for all
LookupKindvariants (ServerFn, Middleware, IsomorphicFn, ServerOnlyFn, ClientOnlyFn)- Environment-specific filtering (Middleware correctly excluded on server)
- Multi-file compilation with proper isolation between files
- Edge cases including whitespace variations, false positives, empty
detectedKinds, and fallback behaviorThe helper function
createFullCompilerappropriately configures environment-specific kind sets. The tests provide strong validation for the new pre-detection optimization that addresses the issue with server-only imports being processed in client builds.packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnFactory.tsx (1)
1-34: Well-structured test input for isomorphic function factories.This file provides comprehensive coverage of factory patterns:
- Full isomorphic implementations (both client and server)
- Partial implementations (server-only, client-only)
- No-implementation factories
- Both function declarations and arrow functions
- Top-level vs factory-returned functions
These patterns effectively test the compiler's ability to handle various isomorphic function creation scenarios.
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnInline.tsx (1)
1-41: Client-side snapshot correctly reflects transformation expectations.The snapshot demonstrates proper client-side transformation:
- Isomorphic function calls replaced with plain functions
- Client implementations preserved (
'client-value','running on client', console.log)- Server implementations successfully stripped
- Server-only functions transformed to no-ops
This validates that server code is properly excluded from client bundles, addressing the core issue in #6185.
packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnInline.tsx (1)
1-55: Comprehensive test input covering inline isomorphic function patterns.The test file validates various inline usage scenarios:
- Assign-then-invoke pattern (Lines 4-9, 12-17)
- Inline IIFE without intermediate variable (Lines 20-24)
- Multiple isomorphic functions in the same scope (Lines 27-37)
- Server-only and client-only implementations (Lines 39-55)
These patterns ensure the compiler correctly handles different ways developers might create and immediately invoke isomorphic functions, which is important for the factory pattern support introduced in this PR.
packages/start-plugin-core/tests/envOnly/snapshots/client/envOnlyFactory.tsx (1)
1-32: Correct client-side transformation for environment-only factories.The snapshot properly demonstrates env-only function handling:
- Server-only factories throw appropriate errors when invoked on client (Lines 2-6, 17-21, 29-31)
- Client-only factories preserve their implementations with console.log and return values (Lines 9-14, 24-26, 32)
- Both function declarations and arrow function patterns are handled consistently
This ensures server-only code cannot execute in client bundles while maintaining client-only functionality, which is critical for the middleware factory support and fix for #6185.
packages/start-plugin-core/tests/envOnly/test-files/envOnlyFactory.tsx (1)
1-31: Well-designed test input for environment-only factory patterns.The test file provides comprehensive coverage of env-only function factories:
- Server-only factories with
createServerOnlyFn(Lines 3-9, 19-22, 30)- Client-only factories with
createClientOnlyFn(Lines 11-17, 24-27, 31)- Both function declaration and arrow function styles
- Factory patterns that return wrapped functions vs top-level definitions
These patterns ensure the compiler correctly transforms environment-specific code for both client and server builds, supporting the fix for platform-specific imports in issue #6185.
packages/start-plugin-core/tests/createMiddleware/test-files/middleware-factory.ts (3)
35-41: LGTM!The top-level middleware is correctly structured with a clear ID and server-side implementation. The logic appropriately demonstrates a simple middleware that logs and proceeds with
next().
21-33: This file is a test fixture excluded from TypeScript compilation (packages/start-plugin-core/tsconfig.jsonexplicitly excludestests/**/test-files/**), so the TypeScript strict mode requirement does not apply. The undefinedgetUser()call is intentional—this fixture tests the middleware compiler's code transformation, not runtime execution. Type annotations are optional here.
3-19: This is a test fixture file excluded from TypeScript compilation and does not require type annotations.The file
middleware-factory.tsis located intest-files/and is intentionally excluded from TypeScript compilation per thetsconfig.json(line:"exclude": ["tests/**/test-files/**"]). It serves as an input fixture to theServerFnCompilertest increateMiddleware.test.ts, where it's compiled and compared against snapshots. The undefinedrateLimitandgetUserfunctions are intentional—they're demonstration patterns within the test fixture, not production code. No type annotations are required for test fixture files that are excluded from the TypeScript compilation pipeline.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (1)
75-81: Consider more precise detection patterns.The current patterns work for fast pre-scanning, but they may produce false positives:
- Line 77:
/createMiddleware/matches bothcreateMiddlewareandcreateMiddlewares- Lines 78-80: Factory name patterns match substrings and could match in comments/strings
Since AST parsing follows pre-detection, false positives are acceptable. However, using word boundaries could reduce unnecessary parsing:
🔎 Suggested refinement for reduced false positives
export const KindDetectionPatterns: Record<LookupKind, RegExp> = { ServerFn: /\.handler\s*\(/, - Middleware: /createMiddleware/, - IsomorphicFn: /createIsomorphicFn/, - ServerOnlyFn: /createServerOnlyFn/, - ClientOnlyFn: /createClientOnlyFn/, + Middleware: /\bcreateMiddleware/, + IsomorphicFn: /\bcreateIsomorphicFn/, + ServerOnlyFn: /\bcreateServerOnlyFn/, + ClientOnlyFn: /\bcreateClientOnlyFn/, }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/start-plugin-core/src/create-server-fn-plugin/plugin.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/start-plugin-core/src/create-server-fn-plugin/plugin.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/start-plugin-core/src/create-server-fn-plugin/plugin.ts
🧬 Code graph analysis (2)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (1)
packages/start-plugin-core/src/create-server-fn-plugin/types.ts (1)
MethodChainPaths(18-24)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (1)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (4)
LookupKindsPerEnv(84-98)KindDetectionPatterns(75-81)LookupKind(35-40)detectKindsInCode(104-120)
⏰ 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). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (13)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (5)
2-7: LGTM! Imports align with pre-detection functionality.The new imports support environment-aware pre-detection and filtering, which addresses the core issue where server-only code was being processed in client builds.
12-19: LGTM! Proper handling of virtual module IDs.The null-byte prefix handling correctly strips Vite/Rollup virtual module prefixes before processing.
21-33: LGTM! Environment-specific filter derivation is correct.The function properly derives transform filters from
KindDetectionPatternsbased on environment, establishing a single source of truth for detection patterns.
99-100: LGTM! Dynamic filter derivation per environment.Correctly derives the transform filter per environment, ensuring only relevant code kinds are processed in each build context.
168-177: LGTM! Pre-detection enables environment-aware filtering.The pre-detection of code kinds before parsing is the key fix for issue #6185. By detecting which kinds are present in each file and filtering by environment, server-only middleware imports will no longer be analyzed during client builds, preventing build failures with platform-specific modules.
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (8)
83-98: LGTM! Environment-specific kind filtering is correct.The environment-specific sets correctly define which kinds should be processed in each build context. Notably,
Middlewareis only processed in client builds (line 86), where it will be transformed to strip server-only code, preventing platform-specific server imports from breaking client builds.
100-120: LGTM! Pre-detection logic is sound.The function efficiently pre-scans code using regex patterns, filtering by environment validity before AST parsing. This optimization reduces unnecessary parsing work and enables environment-aware filtering.
140-148: LGTM! Factory name filtering is appropriate.The set correctly identifies actual factory function names, enabling proper distinction between factory calls (e.g.,
createServerOnlyFn()) and invocations of created functions (e.g.,myServerFn()).
164-177: LGTM! Detection optimization is correct.The function correctly determines whether direct-call candidate detection is needed, enabling performance optimization by skipping unnecessary checks during AST traversal.
179-196: LGTM! Nested candidate detection is properly scoped.The function correctly identifies nested direct-call candidates by checking against known factory names, preventing false positives from function invocations.
198-231: LGTM! Top-level candidate detection is correct.The function properly validates both the call expression structure and its position in the AST, ensuring only top-level variable declarations are considered as direct-call candidates.
424-578: LGTM! Refactored compilation flow with pre-detection support.The updated
compilemethod correctly:
- Accepts optional
detectedKindsfor pre-filtered processing (lines 428, 434)- Implements single-pass traversal for candidate collection (lines 454-496)
- Uses path-based tracking for method chains via
chainCallPathsmap (lines 460-463)- Properly filters candidates by environment-valid kinds (lines 511-513)
- Walks down chains using stored paths for accurate method chain resolution (lines 539-575)
The refactoring enables the environment-aware filtering that fixes the issue where server-only code was processed in client builds.
913-939: LGTM! Method chain candidate detection is efficient.The function correctly identifies method chain patterns using the pre-computed
IdentifierToKindsmap for O(1) lookup, and the simplified boolean return type improves clarity.
fixes #6185
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.