Change fetch method to a string

[tom-sherman]

Relates to #30 and #38

The variant here wasn't gaining any extra type safety and introducing a cost to the bindings where none was needed.

method accepts any arbitrary string and while it's true that some HTTP methods are specced to be in uppercase, the fetch API will normalise them for you.

[TheSpyder]

I'm a little on the fence about this one. While yes the variant wrapper is somewhat of a type flex, it's also nice to have the compiler guarantee that if the method needs to be set, for the vast majority of use cases the string is spelt correctly.

Could we make the default method a polymorphic variant, which achieves the same goal, and have some other way to set the method that takes a string for those edge cases? I'm not sure exactly how this would work short of a duplicate @obj binding, it just seems like a nice compromise.

I'm adding this to the 2.0 release since it's a breaking change. I might start a feature branch for 2.0 so we aren't leaving your PRs open for months; one of the nice things about the way ReScript compiles is dependencies can point to a git branch instead of NPM and still work perfectly.

[tom-sherman]

Could we make the default method a polymorphic variant, which achieves the same goal, and have some other way to set the method that takes a string for those edge cases

@TheSpyder I'm not sure this would work with Request.method, that would still need to return a string which seems inconsistent. The other option is of course to use a polyvar with encode/decode functions, but that doesn't solve the original problem of trying to reduce conversion code.

[TheSpyder]

ok, fair point.

[spocke]

What if we had convenience functions for common methods like makePost, makeGet, makeDelete some of those could also skip having a body property since you can't send that into a get request for example and it would also avoid the weird case where method is a poly variant as input and a string as a property on the object.

It would make the bindings a bit larger since all the properties would have to be duplicated between these functions. Not sure if it's worth it though since having the wrong method will likely not be missed if you make any form of rest api call since it will simply not work.

[tom-sherman]

some of those could also skip having a body property since you can't send that into a get request

Huh, TIL the fetch API enforced that. It's not something that bs-fetch or TypeScript enforces - I'm guessing because it's pretty difficult to make that mistake without your program not working as you mention.

[TheSpyder]

I haven't forgotten about this, but we're getting fairly close to a 1.0 release candidate. Once I'm happy 1.0 is stable (it won't release until ReScript 10 does) I'll make a 2.0 branch and we can start merging PRs like this one to experiment with API changes.

[TheSpyder]

1.0 seems fairly stable but it might be another month before I have time to start the 2.0 branch. Work has been too busy in the lead up to TinyMCE 6.0 to spend any time on this, and I've had a log of personal stuff to deal with so I can't make time in the evenings for it.

[TheSpyder]

Sorry for letting this sit idle for so long. I've had a very busy time both in work and personal life.

ReScript v10 hit alpha today, so I'll make time soon to wrap up the planned activities in version 1.0 and we can begin breaking APIs in earnest while we play with the new features.