openapi-typescript icon indicating copy to clipboard operation
openapi-typescript copied to clipboard

NextJs options added to fetch

Open petercsaki opened this issue 1 year ago • 12 comments

Changes

NextJs uses fetch's options to pass information about caching. These were removed by openapi-fetch.

https://github.com/drwpow/openapi-typescript/issues/1569#issuecomment-1966999115

Checklist

  • [x] Unit tests updated
  • [x] docs/ updated (if necessary)
  • [x] pnpm run update:examples run (only applicable for openapi-typescript)

petercsaki avatar Feb 28 '24 16:02 petercsaki

🦋 Changeset detected

Latest commit: 719185979bf314d339134330a9795ea4fc5e36eb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openapi-fetch Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Feb 28 '24 16:02 changeset-bot[bot]

Thank you for opening! I’m +1 in favor of fixing this issue, and the test is perfect! But I would like to solve this without having any Next.js-specific code in the runtime. This probably means somehow sniffing out RequestInit and passing it along before/after each middleware (maybe we make new Response() from the URL + init each time? Unsure, but there’s probably a creative way to accomplish this).

If we can make that test pass in a generic way without looking for .next, then we’ll not only solve for Next.js; we’ll solve for all other libraries that are experiencing the same issue.

drwpow avatar Feb 28 '24 16:02 drwpow

Yep, I get it. I'll need to think about how to get the parameters that aren't used in creating the Request object and should be passed as the second parameter for fetch.

petercsaki avatar Mar 05 '24 16:03 petercsaki

I have checked the document of the fetch in Next.js and MDN, as well as the codebase of openapi-fetch. After reviewing them, I believe that there may be no need to add an external option to achieve the desired result. Instead, we can simply add the Next.js options as follows:

  const res = await client.GET("/api", { next: {revalidate: 10}})

I haven't tested it yet. Have you tried this @petercsaki

JE-lee avatar Mar 06 '24 07:03 JE-lee

Yes. Openapi-fetch uses them as parameters when creating the Request object:

https://github.com/drwpow/openapi-typescript/blob/9c277fb0a10c3513de46765a4381ccb722a72af4/packages/openapi-fetch/src/index.js#L72C5-L75C7

but the request object only uses the properties of it that it knows about and others are "forgotten". The code uses only the Request object from then on. So any custom properties are ignored.

petercsaki avatar Mar 06 '24 08:03 petercsaki

Yes. Openapi-fetch uses them as parameters when creating the Request object:

https://github.com/drwpow/openapi-typescript/blob/9c277fb0a10c3513de46765a4381ccb722a72af4/packages/openapi-fetch/src/index.js#L72C5-L75C7

but the request object only uses the properties of it that it knows about and others are "forgotten". The code uses only the Request object from then on. So any custom properties are ignored.

Haha, I got what you mean. I have tested some cases. It seems like The Next.js extended Fetch api only takes the second parameter as the cache option. The following code works as well as the cache mechanism.

  const request = new Request('http://localhost:3000/api/v1/space', {
    method: 'GET',
  })
  const res = await fetch(request, { next: { revalidate: 10 } }) 

so you are in the right way. keep going.

JE-lee avatar Mar 06 '24 11:03 JE-lee

Would it be possible to switch NextJsFetchOptions out for AdditionalProps which is defined as an empty object that we spread into the fetch. Then in each case users could ts declare AdditionalProps to what they want ( nextjs props for example ).

declare module 'openapi-fetch' {
    type AdditionalProps {
        next?: { revalidate?: false | 0 | number; tags?: string[] };
     }
}

Just brainstorming.

IngoVals avatar Mar 08 '24 09:03 IngoVals

@IngoVals love that suggestion! That would be a great addition, and I see the value in typechecking additional properties like that. That would be much better than just adding [key: string]: unknown to the shape.

That could either be part of this PR at the author’s discretion, or as a followup.

drwpow avatar Mar 08 '24 15:03 drwpow

Is this even necessary? NextJS uses "ambient modules" to mutate RequestInit, which openapi-typescript already extends for FetchOptions, so I figure this should already work (as long as you've generated Next types at least once, with next lint).

ryami333 avatar Mar 19 '24 15:03 ryami333

How's going this PR?

FreeAoi avatar Apr 28 '24 22:04 FreeAoi

I very wait this PR too ;)

supercute avatar May 02 '24 07:05 supercute

I've abandoned this, sorry. Someone pointed out that you can pass a custom fetch to each request, and that can use next's cahcing. That solution works for us. Creating a generic solution is not in my scope. Feel free to pick this up.

petercsaki avatar May 02 '24 08:05 petercsaki

No worries! I’m going to close this then, but someone else is free to pick this up, provided my restriction up above: the solution should allow for any arbitrary properties, not just .next

drwpow avatar May 02 '24 14:05 drwpow