Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch RequestInit method parameter issue #128

Closed
TomiS opened this issue Jan 4, 2024 · 4 comments · Fixed by #129
Closed

Fetch RequestInit method parameter issue #128

TomiS opened this issue Jan 4, 2024 · 4 comments · Fixed by #129

Comments

@TomiS
Copy link

TomiS commented Jan 4, 2024

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
Copy link
Owner

TheSpyder commented Jan 4, 2024

Yes rescript-lang/rescript#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).

@TomiS
Copy link
Author

TomiS commented Jan 5, 2024

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants