feat: support asynchronous serialization adapter #6503
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hokkyss
changed the title
Contributor
📝 WalkthroughWalkthroughAdds async-capable data primitives and adapters (AsyncFoo/AsyncCar + adapters), extends core serializer with toSerializableAsync support, registers new adapters, adds a server-function route returning class instances, updates SSR pages, and expands E2E tests to validate async serialization and rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Browser
participant Router as Router/Preloader
participant ServerFn as Server Function
participant Adapter as Serialization Adapter
participant Seroval as Seroval Serializer
Client->>Router: Request /server-function/class-instance
Router->>ServerFn: Invoke server handler
ServerFn->>ServerFn: Create Foo (sync) and AsyncFoo/AsyncCar (instances)
ServerFn-->>Router: Return { foo, asyncFoo, asyncCar }
Router->>Adapter: Serialize values
alt adapter implements toSerializableAsync
Adapter->>Adapter: Await async value (e.g. valueAsync)
Adapter-->>Seroval: Return async-serialized payload
else
Adapter->>Adapter: Use toSerializable (sync)
Adapter-->>Seroval: Return sync-serialized payload
end
Seroval->>Client: Send serialized JSON response
Client->>Client: Render resp.foo.value and resp.asyncFoo.echo()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
View your CI Pipeline Execution ↗ for commit 9216260
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@e2e/react-start/serialization-adapters/src/routes/server-function/class-instance.tsx`:
- Around line 18-33: Replace the untyped state "const [resp, setResp] =
useState<any>(null)" with a strongly-typed state derived from the server
function return type; use Awaited<ReturnType<typeof serverFn>> as the generic
for useState so "resp" has the correct shape (including Foo and AsyncFoo
instances) and update any usages accordingly (e.g., where resp?.foo?.value and
resp?.asyncFoo?.echo() are accessed).
In `@e2e/react-start/serialization-adapters/src/routes/ssr/data-only.tsx`:
- Around line 21-22: Typo in the expected async honk state variable: rename
exepctedAsyncHonkState to expectedAsyncHonkState wherever declared/used (e.g.,
the declarations comparing loaderData.asyncCar.singleInstance.honk() at the
sites referencing exepctedAsyncHonkState and any subsequent assertions); update
both occurrences shown (around the first use and the block at lines 55-57) so
the variable name is spelled consistently as expectedAsyncHonkState and all
references (comparisons/expect calls) are adjusted accordingly.
In `@e2e/react-start/serialization-adapters/tests/app.spec.ts`:
- Around line 99-103: The test named 'class instance' is navigating to the wrong
route; update the page.goto call in the test('class instance', async ({ page })
=> { ... }) to load the class-instance page (replace
'/server-function/custom-error' with the correct class-instance route, e.g.
'/server-function/class-instance') so the test exercises the intended page
before calling awaitPageLoaded(page).
- Around line 20-33: The test is reading the sync expected node
(`${id}-car-expected`) for the async assertion; change the selector used to
build expectedAsyncData to the async expected test id
(`${id}-async-car-expected`) so expectedAsyncData corresponds to the async
section being validated (selectors: `${id}-async-car-expected`,
`${id}-async-car-actual`, `${id}-car-expected`, `${id}-async-foo`).
e2e/react-start/serialization-adapters/src/routes/server-function/class-instance.tsx
Outdated
Show resolved
Hide resolved
e2e/react-start/serialization-adapters/src/routes/ssr/data-only.tsx
Outdated
Show resolved
Hide resolved
hokkyss
commented
Jan 25, 2026
hokkyss
force-pushed
the
async-adapters
branch
from
55a6080 to
995f574
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@e2e/solid-start/serialization-adapters/src/data.tsx`:
- Around line 234-250: The test id for the expected Async Car block is
misspelled; update the data-testid on the expected div so it matches the actual
and other files: change the string template from `${id}-casync-ar-expected` to
`${id}-async-car-expected` in the component rendering the expected JSON for
localData.asyncCar.singleInstance (the div near the JSX that references
localData.asyncCar and data.asyncCar).
In `@e2e/vue-start/serialization-adapters/src/data.tsx`:
- Around line 121-123: Operator precedence in makeFoo causes the ternary to only
apply to the left operand of +; change makeFoo to wrap the ternary in
parentheses so the suffix is appended to both branches (i.e. replace new
Foo(typeof window === 'undefined' ? 'server' : 'client' + suffix) with new
Foo((typeof window === 'undefined' ? 'server' : 'client') + suffix)); update the
expression in function makeFoo to match the pattern used by makeAsyncFoo so
server and client both get the suffix consistently.
🧹 Nitpick comments (1)
e2e/vue-start/serialization-adapters/src/data.tsx (1)
102-113: Consider addingtoSerializableAsynctoasyncCarAdapterfor consistency.The
asyncFooAdapter(line 72-78) includestoSerializableAsync, butasyncCarAdapteronly has synchronous serialization. IfAsyncCaris intended to demonstrate async serialization patterns alongsideAsyncFoo, consider adding the async serialization method.If this is intentional (i.e.,
AsyncCardemonstrates that not all "async" adapters need async serialization), this is fine as-is.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@e2e/solid-start/serialization-adapters/src/data.tsx`:
- Around line 49-70: The AsyncFoo class is not exported; update the declaration
of AsyncFoo to be exported so other modules can access it (e.g., change class
AsyncFoo to export class AsyncFoo) while keeping its constructor, internalValue,
getters (valueAsync, value) and method echo unchanged; ensure the symbol
AsyncFoo is available for external instanceof checks and imports.
- Around line 121-123: The makeFoo function currently applies suffix only on the
client due to operator precedence; update the return to parenthesize the ternary
so the suffix is appended in both environments (i.e., wrap typeof window ===
'undefined' ? 'server' : 'client' before adding + suffix) to match makeAsyncFoo
behavior, and apply the same change in the corresponding makeFoo implementations
in the Vue and React serialization-adapters files.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/react-start/serialization-adapters/src/data.tsx (1)
115-173: FixmakeFoosuffix precedence to matchmakeAsyncFoo.Without parentheses, the suffix is only appended on the client branch due to operator precedence. The ternary must be wrapped before concatenation to ensure consistent behavior between server and client.
🧭 Correct concatenation order
export function makeFoo(suffix: string = '') { - return new Foo(typeof window === 'undefined' ? 'server' : 'client' + suffix) + return new Foo( + (typeof window === 'undefined' ? 'server' : 'client') + suffix, + ) }
🤖 Fix all issues with AI agents
In `@e2e/react-start/serialization-adapters/src/data.tsx`:
- Around line 49-79: The test function passed to createSerializationAdapter must
be a TypeScript type predicate so the adapter infers inputs strictly; update the
asyncFooAdapter's test implementation to have the signature (value): value is
AsyncFoo (i.e., return boolean but declared as a predicate) and adjust the
predicate logic to use instanceof AsyncFoo; ensure related functions
toSerializable, toSerializableAsync, and fromSerializable still reference
AsyncFoo to preserve typing.
- Around line 80-113: The test predicate on asyncCarAdapter uses the 'in'
operator unsafely and will throw for null/undefined; update asyncCarAdapter.test
to first guard that the input is non-nullish (e.g., value != null or typeof
value === 'object') before checking '__type' in (value as AsyncCar) and
comparing to 'asynccar', so the test safely returns false for null/undefined
while still recognizing AsyncCar instances created by makeAsyncCar and used with
createSerializationAdapter.
♻️ Duplicate comments (1)
e2e/vue-start/serialization-adapters/src/data.tsx (1)
121-123: Operator precedence still drops the server suffix.Line 122 has the same precedence issue previously flagged; leaving as-is if that test behavior is intentional.
🧹 Nitpick comments (2)
e2e/solid-start/serialization-adapters/src/data.tsx (1)
102-106: Guardtestagainst non-object values and avoidany.
'__type' in valuethrows onnull/primitives. A defensive guard keeps adapters robust if the registry probes arbitrary values, and improves type safety.♻️ Suggested hardening
export const asyncCarAdapter = createSerializationAdapter({ key: 'asyncCar', - test: (value: any): value is AsyncCar => - '__type' in (value as AsyncCar) && value.__type === 'asynccar', + test: (value: unknown): value is AsyncCar => + !!value && + typeof value === 'object' && + '__type' in value && + (value as { __type?: string }).__type === 'asynccar', toSerializable: (car) => ({ make: car.make, model: car.model, year: car.year, }),As per coding guidelines, prefer stricter typing over
any.e2e/vue-start/serialization-adapters/src/data.tsx (1)
102-113: Hardentestto avoidinon non-objects and dropany.Line 104 can throw if the serializer probes primitives or null. Use
unknownand a safe object guard beforein. As per coding guidelines, avoidanyin TS/TSX.♻️ Suggested adjustment
export const asyncCarAdapter = createSerializationAdapter({ key: 'asyncCar', - test: (value: any): value is AsyncCar => - '__type' in (value as AsyncCar) && value.__type === 'asynccar', + test: (value: unknown): value is AsyncCar => + !!value && + typeof value === 'object' && + '__type' in value && + (value as AsyncCar).__type === 'asynccar', toSerializable: (car) => ({ make: car.make, model: car.model, year: car.year, }), fromSerializable: (value: { make: string; model: string; year: number }) => makeAsyncCar(value), })
hokkyss
force-pushed
the
async-adapters
branch
from
379aeac to
4fc9adf
Compare
hokkyss
force-pushed
the
async-adapters
branch
from
0d1e6c1 to
9216260
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@e2e/solid-start/serialization-adapters/src/data.tsx`:
- Around line 72-78: The test callback for asyncFooAdapter currently has an
implicit any; change its signature to a typed type guard such as (value:
unknown): value is AsyncFoo so TypeScript knows the checked type—update the test
in the createSerializationAdapter call (referencing asyncFooAdapter and the test
callback) to use (value: unknown): value is AsyncFoo => value instanceof
AsyncFoo to remove the implicit any and satisfy strict typing.
Comment on lines
+72
to
+78
| export const asyncFooAdapter = createSerializationAdapter({ | ||
| key: 'asyncFoo', | ||
| test: (value) => value instanceof AsyncFoo, | ||
| toSerializable: (foo) => foo.value, | ||
| toSerializableAsync: (foo) => foo.valueAsync, | ||
| fromSerializable: (value) => new AsyncFoo(value), | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -a 'tsconfig.*\.json' -E node_modules -E dist -E build -E .git | head -20Repository: TanStack/router
Length of output: 1518
🏁 Script executed:
cat e2e/solid-start/serialization-adapters/src/data.tsx | head -100Repository: TanStack/router
Length of output: 2234
🏁 Script executed:
cat /home/jailuser/git/e2e/solid-start/serialization-adapters/tsconfig.jsonRepository: TanStack/router
Length of output: 602
Type the test callback to avoid implicit any.
The test callback at line 74 has an implicit any parameter, which violates the "strict": true TypeScript configuration in effect. Apply a type guard pattern:
export const asyncFooAdapter = createSerializationAdapter({
key: 'asyncFoo',
- test: (value) => value instanceof AsyncFoo,
+ test: (value: unknown): value is AsyncFoo => value instanceof AsyncFoo,
toSerializable: (foo) => foo.value,
toSerializableAsync: (foo) => foo.valueAsync,
fromSerializable: (value) => new AsyncFoo(value),
})📝 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
| export const asyncFooAdapter = createSerializationAdapter({ | |
| key: 'asyncFoo', | |
| test: (value) => value instanceof AsyncFoo, | |
| toSerializable: (foo) => foo.value, | |
| toSerializableAsync: (foo) => foo.valueAsync, | |
| fromSerializable: (value) => new AsyncFoo(value), | |
| }) | |
| export const asyncFooAdapter = createSerializationAdapter({ | |
| key: 'asyncFoo', | |
| test: (value: unknown): value is AsyncFoo => value instanceof AsyncFoo, | |
| toSerializable: (foo) => foo.value, | |
| toSerializableAsync: (foo) => foo.valueAsync, | |
| fromSerializable: (value) => new AsyncFoo(value), | |
| }) |
🤖 Prompt for AI Agents
In `@e2e/solid-start/serialization-adapters/src/data.tsx` around lines 72 - 78,
The test callback for asyncFooAdapter currently has an implicit any; change its
signature to a typed type guard such as (value: unknown): value is AsyncFoo so
TypeScript knows the checked type—update the test in the
createSerializationAdapter call (referencing asyncFooAdapter and the test
callback) to use (value: unknown): value is AsyncFoo => value instanceof
AsyncFoo to remove the implicit any and satisfy strict typing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.