[Event] convert syntheticEvent to record by sgny · Pull Request #690 · rescript-react-native/rescript-react-native
closes #688
I added another type parameter to syntheticEvent to handle the additional touchHistory key in responderSyntheticEvent.
Discussion points to settle:
• touchHistory may be included in the record definition, then what previously used to be syntheticEvent('a) becomes syntheticEvent('a, unit) and responderSyntheticEvent('a) becomes syntheticEvent('a, touchHistory). My testing suggests unit is fine as type parameter 'b, but given such events will have undefined as the value for e.touchHistory, option(unit) may be even safer as e.touchHistory == None would hold.
• If touchHistory is not included in the definition, 'b becomes a phantom type to ensure that the touchHistory method only can be applied on syntheticEvent('a, touchHistory). As expected, e.touchHistory is not available.
• Should type responderSyntheticEvent('a) = syntheticEvent('a, touchHistory); still be included and pressEvent remain defined to be of type responderSyntheticEvent('a)? Not needed, obviously, but would keep closer to the JS definitions.
• [@bs.send] to handle keys previously decorated with [@bs.meth] as discussed before.
• [@bs.get] to define getters for all non-method fields to achieve consistency with reason-react. While the record has Js.Nullable.t types for some keys, IMO, using [@bs.return nullable] to return option types is a better API.
An alternative approach may be to define parametrized modules for SyntheticEvent and ResponderSyntheticEvent and then define each event as an instance of either. I think this approach is straightforward, but based on feedback I can work on that instead.