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

Change `Request.referrerPolicy` to a polymorphic variant #66

Closed tom-sherman closed 2 years ago

tom-sherman commented 2 years ago

Relates to #30 and #38

This introduces a minor (IMO) inconsistency with the JS api. In JS-land you can pass referrerPolicy: "", with this PR you must pass either None or omit it.

I did initially have #"" as one of the variants but it surfaced a bug in rescript formatting. See https://github.com/rescript-lang/rescript-compiler/issues/5346

tom-sherman commented 2 years ago

I've just realised that omitting the #"" variant is incorrect as this is a valid referrerPolicy when reading from a Request instance. I'm going to revert the commit where I removed it and mark this PR as WIP as it's blocked by the rescript formatting bug.

Would be great if anyone knew of an alternative solution!

tom-sherman commented 2 years ago

Can't mark as WIP so closing for now.

TheSpyder commented 2 years ago

I'm surprised you don't have access to swap a PR to be a draft PR. I hope we are able to do this one day, I hate having None as a defined constructor anywhere because it can shadow option constructors if open is used incorrectly.