Use a record to bind CombinedError. Prepare for ReScript migration. #230
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With ReScript /
bs-platform
> @8, the syntax for binding JS classes to OCaml classes is no longer supported. In many ways this is a good thing — the old syntax was a bit cumbersome and circuitous. However, we still had ourCombinedError
binding using that syntax, which was always more trouble than it was worth. By the time we receiveCombinedError
, it is an instance not the class itself. Therefore, we can treat it (for our purposes) more or less like a JS object. We can bind this with a record in Reason ✅We also don't need to do any
Js.Nullable
conversion here from what I can tell. Based on the types of instance properties onCombinedError
we would getundefined
rather thannull
in all cases for optional properties. These can be safely compiled usingoption
in Reason. @kitten let me know if that's not the case and I'll add the appropriateJs.Nullable
wrapper.