Eliminate Foreign Data (or get close to it)

[megamaddu]

react-native supports a subset of CSS styles created using the (effectful?) StyleSheet.create function. It'd be awesome to give this a full Union type (as DOM.css should have as well..).

[dwhitney]

this is trivial to add, but I'd like your opinion - many of the types on the fields of the CSS types are Typescript Union types of the form number | string - for such types I've been simply shortening them to string. Thoughts?

[megamaddu]

That makes sense, since the string version can do everything the number one can.

[dwhitney]

I changed the name of this issue to because it's essentially the same as "make better types for events" - I want to fully eliminate all of the foreign data imports and expand the types as much as possible. I have just ~25 types remaining, and I think it's possible.

[dwhitney]

@spicydonuts I wonder if you've encountered this - ImageLoadEventData has a field uri that may be undefined. https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react-native/index.d.ts#L3679

I've used purescript-nullable, but this is undefined? Any suggestions for how to handle it?

[dwhitney]

Looks like there is purescript-undefinable

[megamaddu]

That might work. I usually end up defining an FFI import when I need undefined

[megamaddu]

(there's also a decent chance null will work, as they're usually treated the same in TS)

[dwhitney]

update on this - I've just about got this all worked out in the events branch (bad name of the branch). Hopefully it'll be in a place to merge to master today and then make a release

[dwhitney]

I need some feedback. I'm about ready to merge this, but I'm unsatisfied with the way I've got events working. Take a look at the controlled-input example. In order for that example to work, the persist function/method must be called on the event or accessing the fields will result in an NPE. I could do something like make the NativeSyntheticEvent type opaque and make the record available via some function like persist :: NativeSyntheticEvent -> Effect RecordType. I haven't used React Basic or react enough to really know what the best way to go next is. I'll do that if I don't get any feedback, but I think there is probably a better way.

Thanks!

cc @spicydonuts @jvliwanag @joelmccracken @jkachmar

[jvliwanag]

My proposal is to define NativeSyntheticEvent as a foreign data type:

foreign import data NativeSyntheticEvent :: Type -> Type

From what I understand, the closest definition, say for timeStamp to the behavior of react is:

timeStamp :: forall a. NativeSyntheticEvent a -> Effect (Maybe Number)

So it's an Effect since the timeStamp js field can change value. In particular, if accessed within the handler, it is Just, and Nothing when accessed asynchronously.

But having Maybe is a bit tedious, and we know that if it were accessed within the handler (say logShow evt.timeStamp in the handler), it really should have a value.

So instead of the previous definition of timeStamp, react-basic instead use:

timeStamp :: forall a. EventFn (NativeSyntheticEvent a) Number

and then we have handler:

type EventHandler n = EffectFn1 (NativeSyntheticEvent n) Unit

handler :: forall n a. EventFn (NativeSyntheticEvent n) a -> (a -> Effect Unit) -> EventHandler n

So the idea is you build an EventFn to access the fields you want on the event, and convert it to some a. At this stage, you can expect that all fields of the event are available. Once you've gathered everything you need, and bound them to some value of type a, then you're given the a to perform your side effect.

There are a few caveats though:

The EventFn, handler, merge, etc functions in react-basic are hardcoded against SyntheticEvent which simply is kind Type. The difference with react-native right now, is we know the type of nativeEvent, so it's Type -> Type. So we might have to duplicate variants of these for react-native-basic.

See draft here: https://github.com/jvliwanag/purescript-react-basic-native/commit/93ea11fb07c94afa73ac949df53c5c60af6622a6

[dwhitney]

@jvliwanag this necessitated some pretty nasty type annotations when using capture/merge. I haven't really got time to work on this today. If you have some ideas and want to make a PR, that'd be great!

[megamaddu]

We use the linear EventFn type for this in the DOM implementation: https://github.com/lumihq/purescript-react-basic/blob/master/src/React/Basic/DOM/Events.purs

The machinery lives outisde the DOM modules though, so you can use it here too: https://github.com/lumihq/purescript-react-basic/blob/master/src/React/Basic/Events.purs#L48

It'd be nice to keep that pattern. The linear type makes property access safe (a synthetic event can't leak out into a callback's scope -- values have to be read synchronously).

[megamaddu]

Doh, sorry, @jvliwanag already explained that. I'm not sure a new type is needed for SyntheticEvent, just a React.Basic.Native.Events module similar to the DOM one which defines the valid accessors.

[dwhitney]

The event types in React Native are a bit more exotic/rich than they are in React, so I'd like to keep the NativeSyntheticEvent as @jvliwanag suggests. The only thing that kinda sucks is the need for the type annotations when using merge, but on reflection, I don't think it's all that bad. I'm not sure how often one needs merge, so I think I'll just fill out the rest of the EventFns we need and then merge this