-
-
Couldn't load subscription status.
- Fork 1.3k
feat: add $withType<TFn>() to restrict isomorphicFn
#5652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add $withType<TFn>() to restrict isomorphicFn
#5652
Conversation
WalkthroughAdds type-level APIs to constrain and chain isomorphic functions: new interfaces (ClientImplRequired, ServerImplRequired, IsomorphicFnWithType), a $withType generic on the base returned by createIsomorphicFn(), and corresponding tests, snapshots, docs, and examples exercising the new typed chaining. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Core as createIsomorphicFn()
participant Base as baseFns (has $withType, server, client)
participant Typed as $withType<TFn>()
participant ServerReg as .server(serverImpl)
participant ClientReg as .client(clientImpl)
Note over Core,Base: createIsomorphicFn returns baseFns exposing $withType and registration methods
Dev->>Core: call createIsomorphicFn()
Core-->>Dev: returns Base
Dev->>Typed: Base.$withType<() => R>()
Typed-->>Dev: returns typed chaining object (IsomorphicFnWithType)
Dev->>ServerReg: .server((...args)=>R)
Dev->>ClientReg: .client((...args)=>R)
Note right of ClientReg: chain produces IsomorphicFn<TArgs, R> ready for use
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/{*-start,start-*}/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 0144e3c
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/start-client-core/src/createIsomorphicFn.ts(2 hunks)packages/start-client-core/src/tests/createIsomorphicFn.test-d.ts(1 hunks)packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnDestructured.tsx(1 hunks)packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnDestructured.tsx(1 hunks)packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnDestructured.tsx(1 hunks)
🧰 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-client-core/src/createIsomorphicFn.tspackages/start-client-core/src/tests/createIsomorphicFn.test-d.tspackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnDestructured.tsxpackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnDestructured.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnDestructured.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-client-core/src/createIsomorphicFn.tspackages/start-client-core/src/tests/createIsomorphicFn.test-d.tspackages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnDestructured.tsxpackages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnDestructured.tsxpackages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnDestructured.tsx
🧬 Code graph analysis (2)
packages/start-client-core/src/tests/createIsomorphicFn.test-d.ts (1)
packages/start-client-core/src/createIsomorphicFn.ts (1)
createIsomorphicFn(65-71)
packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnDestructured.tsx (1)
packages/start-client-core/src/createIsomorphicFn.ts (1)
createIsomorphicFn(65-71)
🔇 Additional comments (7)
packages/start-client-core/src/createIsomorphicFn.ts (3)
24-31: Typed server-only surface looks goodNames, variance, and return types align with existing non-typed variant. No issues.
39-47: IsomorphicFnWithType surface is coherentAPI returns typed server/client builders and preserves union until both sides exist. Good.
50-53: $withType signature is spot onGeneric constraint via AnyFn and mapping to Parameters/ReturnType is correct.
packages/start-plugin-core/tests/createIsomorphicFn/test-files/createIsomorphicFnDestructured.tsx (1)
37-40: Good addition to exercise typed fluent chainThis usage cleanly validates the transformer path with $withType before server/client.
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/client/createIsomorphicFnDestructured.tsx (1)
16-17: Client snapshot matches expectationwithTypeRestriction reduces to the client branch. Looks correct.
packages/start-plugin-core/tests/createIsomorphicFn/snapshots/server/createIsomorphicFnDestructured.tsx (1)
16-17: Server snapshot matches expectationwithTypeRestriction reduces to the server branch. Looks correct.
packages/start-client-core/src/tests/createIsomorphicFn.test-d.ts (1)
74-114: Review comment contains incorrect type assertionThe recommended test for
onlyClienthas an incorrect expectation. Based on the type definitions inpackages/start-client-core/src/createIsomorphicFn.ts:
ServerOnlyFnWithTypeextendsIsomorphicFn<TArgs, TReturnType>withTClient = undefined, so return type is correctlyTReturnType | undefined.ClientOnlyFnWithTypeextendsIsomorphicFn<TArgs, TReturnType, TReturnType>, so return type isTReturnType | TReturnTypewhich simplifies toTReturnType(not a union with undefined).The suggested assertion
expectTypeOf(onlyClient).returns.toEqualTypeOf<string | undefined>()contradicts the actual type definition, which already guaranteesstring. The property checks are sound, but the return type expectation for the client-only case needs correction:expectTypeOf(onlyClient).returns.toEqualTypeOf<string>() // not string | undefinedLikely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/start-client-core/src/createIsomorphicFn.ts (2)
32-37: Critical: Type unsoundness persists in ClientOnlyFnWithTypeThis interface still extends
IsomorphicFn<TArgs, TReturnType, TReturnType>, which incorrectly suggests the server branch returnsTReturnTypewhen only the client is implemented. At runtime, server-side calls returnundefinedfor client-only functions. For consistency withClientOnlyFn(line 18) and runtime behavior, this must extendIsomorphicFn<TArgs, undefined, TReturnType>.Apply this diff:
-export interface ClientOnlyFnWithType<TArgs extends Array<any>, TReturnType> - extends IsomorphicFn<TArgs, TReturnType, TReturnType> { +export interface ClientOnlyFnWithType<TArgs extends Array<any>, TReturnType> + extends IsomorphicFn<TArgs, undefined, TReturnType> { server: ( serverImpl: (...args: TArgs) => TReturnType, ) => IsomorphicFn<TArgs, TReturnType, TReturnType> }
66-70: Critical: $withType breaks method chainsThe
$withType: () => {}implementation still returns an empty function, causing runtimeTypeErrorwhen chains like.$withType<...>().server(...)execute before the transformer runs. This was previously identified with confirmed usage sites in test files. The method must return a chainable object matching the same shape as the root.Apply this diff:
export function createIsomorphicFn(): IsomorphicFnBase { return { - $withType: () => {}, + $withType: () => ({ + server: () => ({ client: () => () => {} }), + client: () => ({ server: () => () => {} }), + }), server: () => ({ client: () => () => {} }), client: () => ({ server: () => () => {} }), } as any }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/start-client-core/src/createIsomorphicFn.ts(2 hunks)
🧰 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-client-core/src/createIsomorphicFn.ts
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-client-core/src/createIsomorphicFn.ts
🧬 Code graph analysis (1)
packages/start-client-core/src/createIsomorphicFn.ts (1)
packages/start-client-core/src/index.tsx (2)
IsomorphicFn(10-10)IsomorphicFnBase(13-13)
⏰ 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 (1)
packages/start-client-core/src/createIsomorphicFn.ts (1)
24-30: LGTM: ServerOnlyFnWithType correctly typedThe interface correctly extends
IsomorphicFn<TArgs, TReturnType, undefined>(via the default), meaning server returnsTReturnTypeand client returnsundefined. The.client()method appropriately returnsIsomorphicFn<TArgs, TReturnType, TReturnType>after adding a client implementation.
72ad1dc to
4d75ba6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/start/framework/react/guide/environment-functions.md (1)
86-101: Minor grammar refinement for clarity.Line 88 has a subject-verb agreement issue. The phrase "enforce the type of both the server and client implementations matches" should be "enforce that both implementations match" or restructured for grammatical consistency.
Suggested revision:
- You can use `.$withType<TFn>()` to enforce the type of both the server and client implementations matches the same function signature. + You can use `.$withType<TFn>()` to enforce that both the server and client implementations match the specified function signature.The code example is clear and accurately demonstrates the feature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/start/framework/react/guide/environment-functions.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)
Files:
docs/start/framework/react/guide/environment-functions.md
docs/{router,start}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place router docs under docs/router/ and start framework docs under docs/start/
Files:
docs/start/framework/react/guide/environment-functions.md
⏰ 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 (1)
docs/start/framework/react/guide/environment-functions.md (1)
103-107: Note accuracy confirmed.The note correctly states that TypeScript will enforce both server and client implementations when using
$withType(). This aligns with the PR objectives and provides helpful guidance to users about the type-level requirements.
allows restricting the type on both "branches" in an isomorphic fn.
Notes
$withTypeto signal that it's a special thing, does not have any affect at runtime$withType(), both.server()and.client()must be chained$withTypeextendsundefined, then it should be fine that one of server/client can be omitted? idk...Summary by CodeRabbit
New Features
Tests
Documentation