hono icon indicating copy to clipboard operation
hono copied to clipboard

feat(utils): specify detailed return type for parseBody

Open usualoma opened this issue 1 year ago • 9 comments

What is this?

More restrictive on the type of value returned from parseBody().

const value = (await parseBody(req))['key'] // => string | File
const maybeArray = (await parseBody(req, {all: true}))['key'] // => string | File | (string | File)[]
const maybeObject = (await parseBody(req, {dot: true}))['key'] // => string | File | Object

Specification Changes

In v4.3.x, BodyData was defined as follows

https://github.com/honojs/hono/blob/75a7a09fc548da2a1a8f35431c456922db7d6c76/src/utils/body.ts#L3

For applications using it directly, this change results in the following ((string | File)[] is no longer included)

const data: BodyData = {} // means Record<string, string | File>

To get the same result as in v4.3.x, you need to do the following.

const data: BodyData<{all: true}> = {}

Although there are some changes from v4.3.x as described above, I think it is useful to restrict the type of parseBody(), so I think it is better to include this change.

The author should do the following, if applicable

  • [x] Add tests
  • [x] Run tests
  • [x] bun denoify to generate files for Deno
  • [x] bun run format:fix && bun run lint:fix to format the code

usualoma avatar May 23 '24 00:05 usualoma

Uh, it wasn't easy. I'll think about it a bit.

usualoma avatar May 23 '24 00:05 usualoma

@yusukebe How about this?

usualoma avatar May 23 '24 03:05 usualoma

Also added type to c.req.parseBody() in 3fb0ef8

usualoma avatar May 23 '24 03:05 usualoma

Hi @usualoma !

It would be nice to have the type definition determined by what the value of the option is. But I think there are two problems:

1. Annotations by IDE are not friendly.

As you can see, it's not easy to understand type definitions for the user, though it should annotate string | File | (string | File)[]

CleanShot 2024-05-23 at 15 15 07@2x

CleanShot 2024-05-23 at 15 15 33@2x

2. I intended the first argument for the generic be parsed object types

I designed it so that the first argument of parseBody() generics is the returned type:

const result = await parseBody<{ key1: string, key2: string }>()
// ^ result should be { key1: string, key2: string }

But with this PR, it will be typed for the options. This will break the spec.

https://github.com/honojs/hono/pull/2771/files#diff-fb57c6d8981c47b991f2dc7914e05041c7d0c6651eabc7145562c00a78bc93d0R63-R72


I'm investigating how to fix these problems, but I can't find a good way now 😦

FYI

You may already know that Simply is a very useful utility type even though it can't resolve the problem 1.

https://github.com/honojs/hono/blob/main/src/utils/types.ts#L40

https://github.com/sindresorhus/type-fest/blob/main/source/simplify.d.ts

yusukebe avatar May 23 '24 06:05 yusukebe

@yusukebe Thank you!

  1. I intended the first argument for the generic be parsed object types

Sorry, the very first push, 288e050, had that problem, but I think it was fixed in the subsequent 3b178fc.

I also added an explicit test at https://github.com/honojs/hono/pull/2771/commits/a6c300d8871e51f333a6b2160579cc70b40134ca. That should work, right?

  1. Annotations by IDE are not friendly.

This is indeed true. I don't know how to solve it and will investigate.

usualoma avatar May 23 '24 07:05 usualoma

@usualoma

I also added an explicit test at https://github.com/honojs/hono/pull/2771/commits/a6c300d8871e51f333a6b2160579cc70b40134ca. That should work, right?

Yes! Thanks!

This is indeed true. I don't know how to solve it and will investigate.

The one way is to write the types as very redundancy, but it might not be the best solution. Hmm.

yusukebe avatar May 23 '24 07:05 yusukebe

Hi @yusukebe How about 0deb075? It will appear as follows

I wanted to change the name of the x shown as [x: string], but I couldn't figure out how to contrl the variable name if I wrote [K in keyof T], so I left it as it is, named x.

CleanShot 2024-05-23 at 18 23 01@2x

CleanShot 2024-05-23 at 18 23 14@2x

CleanShot 2024-05-23 at 18 23 27@2x

CleanShot 2024-05-23 at 18 23 40@2x

usualoma avatar May 23 '24 09:05 usualoma

@usualoma

Awesome!! Let's go with this!

yusukebe avatar May 23 '24 09:05 yusukebe

In 1c18227, variable names were changed to appropriate names. My work is now complete!

usualoma avatar May 23 '24 23:05 usualoma

@usualoma Great! Merging now.

yusukebe avatar May 24 '24 02:05 yusukebe