purescript-contrib / purescript-react

React Bindings for PureScript
MIT License
400 stars 65 forks source link

Events #140

Closed ethul closed 6 years ago

ethul commented 6 years ago

Initial work for updating the representation of events. This is still a work in progress, but putting up what I have so far.

Resolves #85 Resolves #135 Resolves #136

@natefaubion @unclechu @arthurxavierx - Any feedback is most welcome.

natefaubion commented 6 years ago

Sorry I missed this. I don't see how this is type safe though. If SyntheticEvent is a raw Record and not an opaque newtype, then it cannot be safe to call the event methods (like preventDefault)? Additionally, some properties like defaultPrevented are mutable (Boolean).

ethul commented 6 years ago

Good points. Maybe the alternative to go the type class direction (https://github.com/purescript-contrib/purescript-react/issues/136#issuecomment-378685611) would be safer. I would definitely be open to writing this that way. What do you think?

natefaubion commented 6 years ago

I think row types are a fine fit, but treating it as a record is not safe for two reasons:

I think you could possibly provide an opaque newtype indexed by fields and provide a general SProxy based getter that works over Eff, with partial applications.

ethul commented 6 years ago

Thanks. Along these lines sounds good. I was testing out the following. Is this kind of like what you are suggesting?

foreign import data SyntheticEvent_ :: # Type -> Type

type SyntheticEventRow r
  = ( bubbles :: Boolean
    , cancelable :: Boolean
    , currentTarget :: NativeEventTarget
    , defaultPrevented :: Boolean
    , eventPhase :: Number
    , isTrusted :: Boolean
    , nativeEvent :: NativeEvent
    , target :: NativeEventTarget
    , timeStamp :: Number
    , type :: String
    | r
    )

type SyntheticUIEventRow r
  = ( detail :: Number
    , view :: NativeAbstractView
    | r
    )

get
  :: forall eff l r s a
   . RowCons l a r s
  => IsSymbol l
  => SProxy l
  -> SyntheticEvent_ s
  -> Eff eff a
get l r = unsafeGet (reflectSymbol l) r

foreign import unsafeGet :: forall eff r a. String -> SyntheticEvent_ r -> Eff eff a

type SyntheticEvent'' r = SyntheticEvent_ (SyntheticEventRow r)

type SyntheticUIEvent'' = SyntheticEvent_ (SyntheticUIEventRow (SyntheticEventRow ()))

bubbles :: forall eff r. SyntheticEvent'' r -> Eff eff Boolean
bubbles = get (SProxy :: SProxy "bubbles")

detail :: forall eff. SyntheticUIEvent'' -> Eff eff Number
detail = get (SProxy :: SProxy "detail")
natefaubion commented 6 years ago

Yes, exactly.

ethul commented 6 years ago

Great! I will send a PR for this, probably tomorrow.

On Sat, Apr 14, 2018 at 16:14 Nathan Faubion notifications@github.com wrote:

Yes, exactly.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/purescript-contrib/purescript-react/pull/140#issuecomment-381357455, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVYy3Z1MmoaMpS23oO5yESx5ua8Si2aks5toli-gaJpZM4TOPz1 .