Skip to content
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

fix(start): preserve multiple values for same key in FormData serialization #3140

Merged
merged 9 commits into from
Jan 12, 2025

Conversation

EskiMojo14
Copy link
Contributor

Also adds Error and FormData to the allowed list of "serializable" values in Typescript, and make createTransformer more helpful with its types.

Copy link

nx-cloud bot commented Jan 10, 2025

View your CI Pipeline Execution ↗ for commit 8289429.

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

☁️ Nx Cloud last updated this comment at 2025-01-12 23:37:45 UTC

Copy link

pkg-pr-new bot commented Jan 10, 2025

Open in Stackblitz

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/@tanstack/arktype-adapter@3140

@tanstack/create-start

npm i https://pkg.pr.new/@tanstack/create-start@3140

@tanstack/create-router

npm i https://pkg.pr.new/@tanstack/create-router@3140

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/@tanstack/eslint-plugin-router@3140

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@3140

@tanstack/react-cross-context

npm i https://pkg.pr.new/@tanstack/react-cross-context@3140

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@3140

@tanstack/react-router-with-query

npm i https://pkg.pr.new/@tanstack/react-router-with-query@3140

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@3140

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@3140

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@3140

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@3140

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@3140

@tanstack/start

npm i https://pkg.pr.new/@tanstack/start@3140

@tanstack/start-vite-plugin

npm i https://pkg.pr.new/@tanstack/start-vite-plugin@3140

@tanstack/valibot-adapter

npm i https://pkg.pr.new/@tanstack/valibot-adapter@3140

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/@tanstack/virtual-file-routes@3140

@tanstack/zod-adapter

npm i https://pkg.pr.new/@tanstack/zod-adapter@3140

commit: 8289429

@EskiMojo14 EskiMojo14 changed the title Tweak serialisation of FormData to preserve multiple values with the same name fix(start): Tweak serialisation of FormData to preserve multiple values with the same name Jan 10, 2025
const createTransformer = <TKey extends string, TFrom, TTo>(
key: TKey,
check: (value: any) => value is TFrom,
toValue: (value: TFrom) => TTo = (v) => v as never,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these being optional seems odd - they're used in all of the current transformers, and I can't think of a situation where i'd want one and not the other, or neither

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

@schiller-manuel schiller-manuel Jan 12, 2025

Choose a reason for hiding this comment

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

Can you please choose different names than TFrom and TTo ? those are usually used in the navigation API for the from / to props

aside from that, LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

inferred type guards weren't introduced until 5.5, and the type tests run 5.2 onwards
packages/react-router/src/transformer.ts Outdated Show resolved Hide resolved
const createTransformer = <TKey extends string, TFrom, TTo>(
key: TKey,
check: (value: any) => value is TFrom,
toValue: (value: TFrom) => TTo = (v) => v as never,
Copy link
Member

Choose a reason for hiding this comment

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

packages/react-router/src/transformer.ts Outdated Show resolved Hide resolved
@SeanCassiere
Copy link
Member

SeanCassiere commented Jan 12, 2025

Please see the CI type error.

Edit: Link - https://cloud.nx.app/runs/5VSpbygVks/task/%40tanstack%2Freact-router%3Atest%3Atypes

@EskiMojo14
Copy link
Contributor Author

EskiMojo14 commented Jan 12, 2025

should be fixed - it's another case of the workspace version of Typescript being 5.7, but the tests requiring all source code work in 5.2 (which makes sense for compiled code, but means you miss out on useful advancements in TS when writing source code)

I already had to address this mismatch here since I was using inferred type guards (which would not have appeared in the final declaration files)

Copy link
Member

@SeanCassiere SeanCassiere left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes being made here.

@schiller-manuel could you confirm the changes regarding the generics are OK?

@SeanCassiere SeanCassiere changed the title fix(start): Tweak serialisation of FormData to preserve multiple values with the same name fix(start): preserve multiple values for same key in FormData serialization Jan 12, 2025
@SeanCassiere SeanCassiere merged commit 72fbd4c into TanStack:main Jan 12, 2025
5 checks passed
@EskiMojo14 EskiMojo14 deleted the serialize-tweaks branch January 13, 2025 11:20
tannerlinsley pushed a commit that referenced this pull request Jan 14, 2025
tannerlinsley added a commit that referenced this pull request Jan 15, 2025
* checkpoint

* checkpoint

* cleanup

* checkpoint

* client babel now works, runtime and rpc injection

* fix: dont touch anything outside of the function

* server functions now import a tsr-serverfn-split file version

* checkpoint

* checkpoint

* fis tests

* It's working!!!

* move serverFnFetcher to start client

* fix: no extra context, throw errors from middleware

* docs: add custom link example for mantine (#3033)

* ci: apply automated fixes

* fix(react-router): make sure full matches are passed into route functions (#3039)

* release: v1.91.3

* docs: minor typo in code-based-routing.md (#3043)

* feat(start): create-start cli (#2920)

* create start cli

* update lockfile

* ci: apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* docs: Minor typo in middleware.md (#3042)

* release: v1.92.0

* refactor(react-router): expose `scrollBehavior` on the `ScrollRestoration` component (#3053)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Sean Cassiere <[email protected]>

* release: v1.92.1

* fix(start): returning `null` from server functions (#3048)

Co-authored-by: SeanCassiere <[email protected]>

* release: v1.92.2

* fix: allow serverFn errors to also have context (#3037)

* fix: allow serverFn errors to also have context

* fix: allow errors, undefined from server functions

* remove logs

* fix: transformer and tests

* ci: apply automated fixes

* no logs

* fix conficts

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* release: v1.92.3

* move directive plugin to own package

* fixes, tests

* suggested fixes from sean

* fix tests

* fix: import specifier

* fix: dev imports

* fix: simpler config, better logging, more reliable plugin coordination

* fix: update test snapshots

* fix: non-split directives, clean up compilers, logging, and

* fix: production server fn builds

* feat(start): explicitly enable and disable route generation

* fix: $fetch during dev

* ci: apply automated fixes

* Update comparison.md

* docs: remove duplicate word (#3125)

* docs: remove duplicate word

* docs: correct typo in learn-the-basics.md

* chore: add skipLibCheck to examples (#3134)

* docs: Typo issue - getParent Does not Exists in type RouteOptions (#3131)

* docs: Fix notFound import (#3133)

* docs: fix dead link and typo in guide (#3136)

Typo introduced in #2963.

* fix(react-router): separate internal and user provided history state (#3119)

* release: v1.95.2

* fix(router-generator) use explicit routes export by default (#3109)

Co-authored-by: Vladimir Panov <[email protected]>

* fix(start): serialize `FormData` for server function input (#3138)

* release: v1.95.3

* test(react-router): check `FormData` serialization in transformer (#3139)

* docs(router): missing character in code-splitting guide (#3143)

* fix(create-start): readme changes and little fixes (#3142)

* release: v1.95.4

* fix(start): preserve multiple values for same key in `FormData` serialization (#3140)

Co-authored-by: SeanCassiere <[email protected]>

* release: v1.95.5

* docs(migrate): correct typo in migration documentation (#3153)

* docs(start): fix `FormData` server functions examples (#3062)

Co-authored-by: Sean Cassiere <[email protected]>

* docs(router): require the use of React's `createRoot` function (#3151)

Co-authored-by: Sean Cassiere <[email protected]>

* fix: order of injected scripts (#3158)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* release: v1.95.6

* perf(start): reduce instantiations of server fn and middleware (again) (#3168)

* perf: reduce instantiations of server fns

* perf(start): reduce instantiations of middleware

* release: v1.95.7

* fix imports

* fix: build test

---------

Co-authored-by: Youssef Benlemlih <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Manuel Schiller <[email protected]>
Co-authored-by: Tanner Linsley <[email protected]>
Co-authored-by: AlexisPin <[email protected]>
Co-authored-by: timoconnellaus <[email protected]>
Co-authored-by: Arnaud Kleinpeter <[email protected]>
Co-authored-by: Joshua Knauber <[email protected]>
Co-authored-by: Sean Cassiere <[email protected]>
Co-authored-by: alakhpc <[email protected]>
Co-authored-by: Mark Vu <[email protected]>
Co-authored-by: Christopher Horobin <[email protected]>
Co-authored-by: Muthu Kumar <[email protected]>
Co-authored-by: Jeff Hertzler <[email protected]>
Co-authored-by: Philippe Serhal <[email protected]>
Co-authored-by: Andrei <[email protected]>
Co-authored-by: Volodymyr <[email protected]>
Co-authored-by: Vladimir Panov <[email protected]>
Co-authored-by: pierluigigiancola <[email protected]>
Co-authored-by: Tom Ballinger <[email protected]>
Co-authored-by: Ben Durrant <[email protected]>
Co-authored-by: Lukas Frey <[email protected]>
Co-authored-by: John <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants