The code looks fine to me but I'm skeptical this will be reliable in practice, as it so heavily depends on TypeScript, which is often lagging or incomplete.

if I understand correctly, Next.js has a monkeypatched Request class that exposes request.next, and Ky is correctly passing that on, but you still want us to pass options.next when we call fetch(request, options) even though the request already has it? Why would that even be necessary? Does their fetch() just ignore the request properties when the options argument is also present? If so, that sounds like a design flaw in their fetch() implementation.

IMO, if they are going to add a custom option, they need to support passing it the same way as other options, through fetch(url, options), fetch(request, options), or fetch(request). All of those should work, with the option in either argument (or both). And if they do, then Ky's existing implementation should also work.

I am not sure what you mean by

heavily depends on TypeScript, which is often lagging or incomplete

I don't think anything here is related to typescript. The question is basically "which unknown properties should be passed on to the wrapped fetch's second options parameter rather than the first request parameter, to deal with frameworks which accept additional parameters via options but not via request". The two examples of such frameworks we have is Bun (with the proxy parameter as described in #516) and Next.js with the next parameter (which was the motivation for my issue #541).

The initial fix for the Bun issue - #536 - attempted to use the heuristic of assuming that only properties that are not in Request are extra properties, to avoid hardcoding a list of "known" parameters - but this already caused the problem reported by #539 - and the fix in #540 was to revert to having a hardcoded list. And my point is simply that with that done, the extra check that the property is also not in Request is unneeded.

I think your point that Next.js and Bun could have chosen to support to support those extra params in request as well as options seems reasonable to me - Next.js seems to have many issues with inconsistent implementation of the various fetch / Request parameter permutations. But you already did choose to implement the earlier fixes rather than take the stand that it should be on Bun to address this in the case of #516 - this is just actually trying to make the fix also work for Next.js case.

I should note that I ended up switching over to using wretch in the meantime. This is no knock on ky - I think both libraries are good, and I don't have a strong preference in terms of API design. But this really is kind of a blocker - those Next.js revalidation props are pretty key to making the most of their new app router's caching system - and I needed to keep making progress on what I was working on.

So you can feel free to close the issue/PR if you like - I just wanted to explain my thinking above, and sorry for being long-winded

I don't think anything here is related to typescript.

This PR removes code that is meant to be a backup in case the requestOptionsRegistry fails to catch options that are handled by the Request class. Since we're leaning on TypeScript to determine the completeness of the requestOptionsRegistry, this PR by definition makes us more reliant on TypeScript, as we will no longer have the backup mechanism that dynamically checks the request object.

I would be okay with that if TypeScript was always up to date, but in my experience, people start using new JavaScript APIs (including fetch() options) long before they are added to the TypeScript definitions.

In case it's not obvious, the reason we want to maintain this strict separation of "request" options and "unknown" options, ideally with no duplication between them, is because anything that gets passed in the second argument to fetch() overrides the first argument. So we use the request object as a single source of truth, make any changes to headers and other options there and then don't pass those options in the second argument to fetch(), in order to avoid accidentally overriding our own work.

Anyway, if you do end up having some time and get the tests passing, I'll see if I can come up with a solution that doesn't overly rely on TypeScript while also not breaking #541.

Bump :)

Would be awesome if we can land this. Is there anything that is blocking it and we can help with?