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

feat: add runtime typesafe body and query parameters helpers #431

Closed
wants to merge 5 commits into from

Conversation

Hebilicious
Copy link
Member

@Hebilicious Hebilicious commented Jul 8, 2023

  • feat: add formdata and request helpers
  • chore: rename to eventToRequest and readFormData
  • feat: add readBodySafe and getQuerySafe helpers

πŸ”— Linked issue

Resolves #419
Resolves #402

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

(This PR includes #421)

This PR ads two new helpers to H3

  • readBodySafe
  • getQuerySafe

Both of these helpers accept a https://github.com/decs/typeschema schema as the 2nd argument, which is compatible with zod, yup, joi, superstruct, typia, runtypes, arktype and custom validation function.
readBodySafe implementation will use FormData for x-www-form-urlencoded, and then rely on safeDestr for body parsing before passing it to the schema validation. By design, both these function will throw on runtime validation errors.

It would also be possible to take https://github.com/decs/typeschema and extract its logic into our own internal helpers or UnJS, as it's a very tiny and clever library (50 LOC). We could also look into creating a unjs version of this library that fits our needs in the future.

I'm looking for feedback for alternative APIs.

πŸ“š Usage

Here's an example with a zod schema.

  eventHandler(async (event) => {
    const schema = z.object({
      firstName: z.string(),
      lastName: z.string(),
    });
    const data = await readBodySafe(event, schema);
    expectTypeOf(data).toMatchTypeOf<{
      firstName: string;
      lastName: string;
    }>();
    return { ...data };
  })

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@pi0
Copy link
Member

pi0 commented Jul 10, 2023

(This PR includes #421)

Please always target PRs against main πŸ™πŸΌ

@pi0 pi0 marked this pull request as draft July 10, 2023 12:16
@Hebilicious Hebilicious added pending and removed enhancement New feature or request labels Jul 11, 2023
@Hebilicious
Copy link
Member Author

(This PR includes #421)

Please always target PRs against main πŸ™πŸΌ

I'm gonna put this as pending until #421 is merged

@Hebilicious Hebilicious marked this pull request as ready for review July 25, 2023 07:43
@Hebilicious
Copy link
Member Author

@pi0 I've un-drafted this now that readFormData has landed πŸ‘πŸ½

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #431 (46300b5) into main (78aec63) will increase coverage by 0.45%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##             main     #431      +/-   ##
==========================================
+ Coverage   78.37%   78.82%   +0.45%     
==========================================
  Files          26       27       +1     
  Lines        2793     2862      +69     
  Branches      407      418      +11     
==========================================
+ Hits         2189     2256      +67     
- Misses        604      606       +2     
Files Changed Coverage Ξ”
src/utils/body.ts 94.88% <93.54%> (-0.26%) ⬇️
src/utils/internal/validation.ts 100.00% <100.00%> (ΓΈ)
src/utils/request.ts 98.64% <100.00%> (+0.19%) ⬆️

@pi0
Copy link
Member

pi0 commented Jul 25, 2023

While I love the typeschema lib idea, it is beyond 50 LOC when bundled as it registers adapters. I agree with you, we might make an unjs similar library in order to achieve this because we also need validation fo db0, etc. (It might be a major version of unjs/untyped ~> unjs/untyped#101)

I think for this to move forward, we need to fully decouple the validation function from the underlying library. So the readValidatedBody requires a validate function which can be wrapped into an adaptor via a subpath h3/zod, h3/typeschema or an external h3-zod, h3-typeschema to integrate with those libs.

@Hebilicious
Copy link
Member Author

While I love the typeschema lib idea, it is beyond 50 LOC when bundled as it registers adapters. I agree with you, we might make an unjs similar library in order to achieve this because we also need validation fo db0, etc. (It might be a major version of unjs/untyped ~> unjs/untyped#101)

I think for this to move forward, we need to fully decouple the validation function from the underlying library. So the readValidatedBody requires a validate function which can be wrapped into an adaptor via a subpath h3/zod, h3/typeschema or an external h3-zod, h3-typeschema to integrate with those libs.

The first version that I tried had a very simple implementation that worked great : https://github.com/decs/typeschema/blob/v0.1.3/src/assert.ts

Maybe we could just inline that logic for now and support slightly less schemas ?

@pi0
Copy link
Member

pi0 commented Jul 25, 2023

I prefer to do it step by step and with zero dependencies added now. Such checks need proper test suite also. So let's keep this PR minimal to introduce bare validated utils first and iterate over. (people can pass schema.[method] as validate function)

@decs
Copy link

decs commented Jul 25, 2023

hey, folks! typeschema maintainer here

we started with a simple logic, inspired by tRPC, but quickly noticed that it wouldn't support other libraries where the validation function lives outside of the schema object (like io-ts, ajv). so we pivoted to the adapter + optional peer dependencies model, and now we're on track to support all major libs this week. and indeed there's a tradeoff: the architecture change increased the LOC.

I can think of 2 alternatives to adding typeschema as a direct dependency, if the LOC is a concern:

  1. making it an optional peer dependency. try dynamically importing typeschema (falling back to no schema validation if not installed) and instructing users to install it on their own
  2. or supporting custom validators with this format: <T>(data: unknown): Promise<T> | T, then asking users to independently install typeschema and use createAssert(schema) (which generates a function on this format)

@pi0
Copy link
Member

pi0 commented Jul 25, 2023

Hi dear @decs thanks for your genius idea for simple util and nice library and explanations.

Indeed the schemas with decoupled validator util is another concern we have to take into account.

I think we still want to move forward with a direct option for custom validators and make wrapper composables in order to provide integrations in next strps (if unified, even better!).

I have opened a discussion in unjs/untyped#101 to brainstorm how to make it a generic utility so we can use it across unjs ecosystem, if you like to weight in there and put any ideas how to do this.

@pi0
Copy link
Member

pi0 commented Jul 26, 2023

#459

@pi0 pi0 closed this Jul 26, 2023
@pi0 pi0 deleted the read-safe-body branch August 1, 2023 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[helpers] Native Request and FormData helpers Type and Runtime validated request utils
3 participants