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

Start server function validator only supports the ValidatorFn shape (so things like Zod don't work as expected) #2759

Closed
marbemac opened this issue Nov 14, 2024 · 15 comments
Labels
start Everything about TanStack Start

Comments

@marbemac
Copy link
Contributor

marbemac commented Nov 14, 2024

Which project does this relate to?

Start

Describe the bug

On the Start v1.81.7, the server function validator middleware does not actually support the options described by its typings.

It accepts AnyValidator (https://github.com/TanStack/router/blob/main/packages/react-router/src/validators.ts#L62-L67).

However, the actual implementation assumes the validator is the ValidatorFn shape (https://github.com/TanStack/router/blob/main/packages/start/src/client/createServerFn.ts#L325-L330).

Looks like nextMiddleware.options.validator in that createServerFn.ts has the any type - any ways to cleanup the types in there as well? If nextMiddleware.options.validator were set to AnyValidator, the issue would have been surfaced right away.

Your Example Website or App

NA

Steps to Reproduce the Bug or Issue

Attempt to use zod, or zod + @tanstack/zod-adapter, with a server function validator and it'll throw.

Expected behavior

The server function validator supports the validator shapes described in @tanstack/zod-adapter.

Screenshots or Videos

No response

Platform

  • OS: mac
  • Browser: chrome

Additional context

No response

@schiller-manuel
Copy link
Contributor

do you have a minimal example that we can use to debug this?

@marbemac
Copy link
Contributor Author

marbemac commented Nov 14, 2024

@schiller-manuel looks like router already has a validateSearch function that handles the various validator shapes, but it's not exported. Could maybe re-use that?

Re example - can use the @tanstack/zod-adapter in any of the examples. I reproduced it by changing https://github.com/TanStack/router/blob/main/examples/react/start-basic-react-query/app/utils/posts.tsx#L28 to .validator(zodValidator(z.string())) (and add zod + @tanstack/zod-adapter to the package.json of course). Then fire up the example and navigate to the post route.

@schiller-manuel
Copy link
Contributor

thanks.
yes we will probably reuse this from router.
if you want, you can also create a PR for this

@marbemac
Copy link
Contributor Author

marbemac commented Nov 14, 2024

Happy to - will open it shortly. Just wanted to confirm ya'll are OK exporting and re-using that function from router - exporting it kind of makes it part of the "public" api surface in a way.

@schiller-manuel
Copy link
Contributor

exporting it does not really mean it is public API IMO. it's public when we add API docs (which we don't do here)

@tannerlinsley
Copy link
Collaborator

I just pushed a "mostly" fix for this, but as we've been discussing the adoption of the "Standard Validation" schema internally, it might need to be massaged for backwards compatibility.

@marbemac
Copy link
Contributor Author

Ah ok I see you copied over the validateSearch code rather than re-use from router - is intent over time for the validation interface to remain consistent across router and start? If so, maybe re-use rather than copy so that they don't diverge and to make that connection clear?

Either way, the pushed change did resolve the issue :).

There's a type error though that will likely block the build process.

@marbemac
Copy link
Contributor Author

Getting familiar with the typings for functions / middleware's will take me longer than I have right now, but just one more pitch to somehow get things like nextMiddleware.options.validator typed correctly if possible - even though it's internal, would help to catch this kind of issue in the future especially as there is so much code churn happening in Start atm. I know ya'll have a lot on your plate though and it's prob low priority which is ok.

@tannerlinsley
Copy link
Collaborator

Yeah, I'm currently upgrading TanStack.com to this new version, so I feel the same pain. @chorobin is on vaycay, but is the most familiar with this code. I'm sure he can whip up a proper fix when he gets back. Let's let him rest. To your other question, yes, the plan is to support the standard validation schema format and also provide some fallback adapters that not only work across Start and Router, but also Form, etc.

@tannerlinsley
Copy link
Collaborator

Also, if you know how to fix the types, be my guest and PR. It's a high priority, but I just have so much on my plate right now I need to personally defer.

@marbemac
Copy link
Contributor Author

Sounds good! Re the types I'll see if I can take a look but at first glance it looks like there's a high up front cost just to get familiar with the approach to types in router/start. Might not have enough time today, but perhaps in the next few days.

@SeanCassiere SeanCassiere added the start Everything about TanStack Start label Nov 15, 2024
@schiller-manuel
Copy link
Contributor

is this resolved with the latest releases?

@marbemac
Copy link
Contributor Author

Yes, but Tanner mentioned "I'm sure he can whip up a proper fix when he gets back" so I wasn't sure if the current fix is temporary.

@schiller-manuel
Copy link
Contributor

ok then let's keep this open until @chorobin is back

@schiller-manuel
Copy link
Contributor

@chorobin told me to close this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
start Everything about TanStack Start
Projects
None yet
Development

No branches or pull requests

4 participants