Changes type of retry.methods option to HttpMethod

[vokrik]

I learned the hard way that the methods need to be lowercase (and kept wondering why the requests are not retried). This change should prevent that mistake

[vokrik]

Had to move this to common so that there is not a circular dependency between retry.ts and options.ts

[sholladay]

Circular dependencies are fine in ESM.

[vokrik]

Right, I think this is just a tick from some past projects :) I'll move the HttpMethod back to options.ts

[vokrik]

For some reason, in constants.ts there is an exported constant containing methods without 'options' and 'trace'

// source/core/constants.ts
...
export const requestMethods = ['get', 'post', 'put', 'patch', 'head', 'delete'] as const;

I guess we want to keep split the methods that the user does proactively (rather than the browser doing automatically) and all available methods.

The default retryMethods however contain also options and trace as a default:

// source/utils/normalize.ts
const retryMethods: Array<HttpMethod> = ['get', 'put', 'head', 'delete', 'options', 'trace'];

[sindresorhus]

I appreciate you working to prevent the uppercase method mistake. However, hardcoding HttpMethod as a union type would break extensibility. There are almost 40 registered HTTP methods in the IANA registry (not just 8). The Fetch API also allows arbitrary custom methods, and users might want to retry them.

Instead, we should use the LiteralUnion pattern that Ky already uses elsewhere:

methods?: Array<LiteralUnion<HttpMethod, string>>;

This provides autocomplete for common methods while still accepting any string, maintaining both type safety and extensibility.

[vokrik]

I appreciate you working to prevent the uppercase method mistake. However, hardcoding HttpMethod as a union type would break extensibility. There are almost 40 registered HTTP methods in the IANA registry (not just 8). The Fetch API also allows arbitrary custom methods, and users might want to retry them.

Instead, we should use the LiteralUnion pattern that Ky already uses elsewhere:

methods?: Array<LiteralUnion<HttpMethod, string>>;

This provides autocomplete for common methods while still accepting any string, maintaining both type safety and extensibility.

Ok, that makes sense.. In that case, would it make sense to you if I lowercased the passed methods in normalize.ts so that it's bulletproof? (Added proposal as a last commit)

[LinusU]

Was this change meant to be included?

[vokrik]

nope, sorry, was removing a typecast and my IDE left the braces