TheSpyder / rescript-webapi

ReScript bindings to the DOM and other Web APIs
http://tinymce.github.io/rescript-webapi/api/Webapi/
Other
149 stars 36 forks source link

Fetch RequestInit method parameter issue #128

Closed TomiS closed 9 months ago

TomiS commented 9 months ago

Hey, I'm not sure if this is a ReScript 11 thing or something else, but in order to make fetching work properly, I needed to change this line and this line in such way that neither have underscore as prefix

e.g.

~_method: string=?,

=>

~method: string=?,

The current implementation does not work when using DELETE as method (at least). It causes an error TypeError: Failed to construct 'Request': Request with GET/HEAD method cannot have body.

TheSpyder commented 9 months ago

Yes https://github.com/rescript-lang/rescript-compiler/pull/6354 removed _ mangling; in earlier compiler versions the _ prefix was introduced as a way to allow binding to JS names that were reserved words in ReScript. Now instead of _method compiling to method it's emitting _method. As a result your fetch call is not actually specifying a method and falling back to the default.

I'll try to find some way to fix this without breaking ReScript 10 compatibility. It would be a shame to need a breaking version change just for v11 support.

If I can't make it work I might just pull fetch out and link to rescript-fetch instead (the fetch support in webapi was pulled from glenn's bs-fetch which I thought was abandoned).

TheSpyder commented 9 months ago

Other locations: https://github.com/tinymce/rescript-webapi/blob/5219ef22571c7e79ac221d049c1ad19546270c99/src/Webapi/Webapi__File.res#L15 https://github.com/tinymce/rescript-webapi/blob/5219ef22571c7e79ac221d049c1ad19546270c99/src/Webapi/Webapi__Blob.res#L30

And the tests for both

TomiS commented 9 months ago

Sounds good. Thanks for clarifying.

Edit: Though I think _type is still a keyword that needs to be mangled even in RS 11, right?

TheSpyder commented 9 months ago

I didn't see your edit. That's... probably something I should've reported to the compiler team. Hmm.