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

Bindings cleanup v2 #105

Merged
merged 3 commits into from
Aug 14, 2020
Merged

Bindings cleanup v2 #105

merged 3 commits into from
Aug 14, 2020

Conversation

tsnobip
Copy link
Contributor

@tsnobip tsnobip commented Aug 11, 2020

Thanks to bs.as allowing undefined in BS 8.2.0 I changed the API for unsetValue functions to setValueToNull / SetValueToUndefined which is a bit nicer to use I think and allows 0 runtime cost bindings.

I also cleaned the bindings again, used the naming convention we agreed on for internal raw types and functions, and use bs.obj and bs.deriving abstract instead of Js.t objects to have a more robust type-check and to avoid undefined fields in objects.

I think we're pretty close to not having to clean objects to remove undefined fields, maybe all that's left is to change the type of connections.

The next round of cleanup could be focused on reducing the number of Curry calls in the generated JS.

This breaking change allows to remove runtime cost for these bindings
and makes the API clearer  (removed a variant type)
Names of raw JS functions, raw JS bindings only used internally
or raw JS types only used in internal bindings
are now ending with 'Raw'

renamed _convertObj to convertObj

Removed _cleanObjectFromUndefined and _cleanVariables from .rei file
since they were not used

switched from let to external in ReasonRelay.rei
for bindings without runtime cost
This allows a more robust type-check and
avoids undefined fields in JS objects
@zth
Copy link
Owner

zth commented Aug 14, 2020

This is great, thank you! 😍

@zth zth merged commit e7cf0b3 into zth:master Aug 14, 2020
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 this pull request may close these issues.

2 participants