Comments (10)
@kdelmonte I am following the thread you linked as well - since I am one of the people affected by that issue. It looks like Next.js is working on a fix, but I am waiting to see how it plays out - the linked PR does not yet have a fully working fix.
However, I don't think that issue is related to this one - other than the coincidence that both affect edge runtime on Next.js dev server.
the problem described in that issue has to do with the fact that when you do something like
const req1 = new Request('https://example.com', {
method: 'POST',
headers: { 'X-Example-Header': 'Foo' }
});
const req2 = new Request(req1);
req2
should carry over properties like method
, headers
, etc from req1
, but for edge runtime in nextjs dev server, it does not. This ends up breaking ky when you do something like ky.post('https://example.com', { json: { foo: 'bar' } })
because ky's implementation for the json
shortcut relies on the Request
copy constructor.
The problem described in this issue is with passing additional properties to fetch
that neither ky
nor the web-standard fetch
knows about. something like ky.get('https://example.com', { next: { revalidate: 10000 } })
. This is important to be able to use the cache/revalidation capabilities of Next.js's app router.
This was previously reported in #531 and was attempted to be fixed in #536. And the fix mostly works - except in edge runtime in nextjs (it definitely does not work in edge with dev server. and I cannot test it with edge on Vercel prod, because they are currently having issues deploying edge functions). But the reason does not have anything to do with the nextjs issue above. As I mentioned in my original post, I think it has to do with the way #536 is implemented. Specifically in https://github.com/sindresorhus/ky/pull/536/files#diff-1d28e040cea335f5a22aa9c3f678f5d00b353e3ce966dc0978a91fe56f240b04R301 it only passes to fetch
nonRequestOptions
- which it computes in https://github.com/sindresorhus/ky/pull/536/files#diff-4a531f309dc0b2c2d0c4d04fae930a1731ebdc2cd5cfd204a1e98ea5a71e3f2fR10 as any keys that are not the known special ky-only properties, and not properties present in Request
. My theory (which I think is supported by https://github.com/vercel/next.js/blob/canary/packages/next/src/server/web/sandbox/context.ts#L370) is that Next.js patches Request
to also have a next
property. As a result next
is not included in nonRequestOptions
and therefore not passed to fetch
.
You can prove this with something like this:
const customFetch: typeof fetch = async (request, init) => {
console.log('init is ', init);
return fetch(request, init);
};
ky.post('https://example.com', { next: { revalidate: 1000 }, proxy: 'https://someproxy.com' });
if I run this in edge runtime I get
init is { proxy: 'https://someproxy.com' }
whereas in node I get
init is { next: { revalidate: 1000 }, proxy: 'https://someproxy.com' }
So the problem is very specific to the next
option. And I dont think it will be fixed by the Next.js issue referenced in your comment above.
One possible solution could be if we could only check the "declared" keys of the original Request
type when computing nonRequestOptions
. otherwise I guess they would have to be hardcoded.
Let me know if any of the above doesn't make sense.
from ky.
Thank you for the clarification. I will investigate.
from ky.
One other thing worth mentioning is that it's a bit unergonomic to pass those unknown options to ky, since if I pass them directly, the type checker complains.
I either have to do one of two things
ky(someUrl, { next: { revalidate: 10000 } } as any);
//or
const options = { next: { revalidate: 10000 } };
ky(someUrl, { ...options });
which work but are not ideal.
from ky.
// @kdelmonte
from ky.
I'll take a look.
from ky.
Relates to:
vercel/next.js#41531
vercel/edge-runtime#688
from ky.
@dkokotov I've been following this thread and it seems like this this is a next.js issue that shouldn't require any modifications to ky. Please clarify when you can.
Also, regarding the typescript error that you are getting, please provide a reproduction repository.
from ky.
@kdelmonte (or @sindresorhus ) not sure if you've had a chance to look yet, but please let me know what you think of the approach in #542
from ky.
hey @dkokotov, I just took a look at your PR and I think it does make sense that we would not need the !(key in request)
check after the work that was done as part of #540. I see that you do have some failing tests though.
from ky.
To be honest I had some weird behaviors with the tests. When they ran on my fork they initially hung but then passed (https://github.com/dkokotov/ky/actions). so not sure why they are failing in the context of the PR. And I had trouble making them run locally.
As I mentioned in my comment on the PR (#542 (comment)) I ended up switching to a different library just cause I needed to make progress on what I had been trying to use this for. so I probably won't have time to chase down the test issues. Feel free to either take the issue / PR over or to close it.
from ky.
Related Issues (20)
- "fetch() request with GET/HEAD/OPTIONS method cannot have body" in Bun HOT 2
- retry is wrongly counting HOT 1
- Error: `unsupported BodyInit type` on some runtimes HOT 1
- Ky does not pass custom options down to the native fetch HOT 4
- is there a way to prevent HttpError when response is not in range HOT 2
- POST request fails in Astro.js API route HOT 5
- Header not set on node version 18.18.2 HOT 14
- [help] trying to understand why headers are not set properly HOT 1
- 1.1.1 breaks multipart form boundary HOT 6
- Tests are hanging HOT 2
- Best way to handle Fetch errors such as Failed to fetch, NetworkError when attempting to fetch resource, etc. HOT 1
- Retries on timeout HOT 1
- TypeError: signal.throwIfAborted is not a function HOT 9
- Support for SSL client certificate in Nodejs undici HOT 1
- How to abort retry before hitting the retry limit?
- Cannot use import statement outside a module, Jest HOT 1
- Types for headers HOT 2
- How to handle HTTP Delete Method (204) with ky?
- [BUG] When Response is a string(not valid JSON), `json` method occurs error
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ky.