-
Notifications
You must be signed in to change notification settings - Fork 64
Revamp events #136
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
Comments
One thing to consider is that events in React are pooled by default, and it's not safe to hold on to references of them without taking precautions (https://reactjs.org/docs/events.html#event-pooling). |
Good to know. Thanks for the link. |
Apologies on the delay here. I was thinking of a couple of options in terms of implementation. Option 1 - TypeclassesThe main idea here is to define foreign data for each type of event, along with a typeclass. If an even type has an instance of the typeclass then the associated functions could be called for the given event type. For example, all of the events types would have an instance of foreign import data SyntheticEvent :: Type
class IsSyntheticEvent a where
toSyntheticEvent :: a -> SyntheticEvent
instance isSyntheticEventSyntheticEvent :: IsSyntheticEvent SyntheticEvent where
toSyntheticEvent = id
instance isSyntheticEventSyntheticClipboardEvent :: IsSyntheticEvent SyntheticClipboardEvent where
toSyntheticEvent = unsafeCoerce
...etc
bubbles :: forall a. IsSyntheticEvent a => a -> Boolean
bubbles = (#) _.bubbles <<< unsafeCoerce <<< toSyntheticEvent
cancelable :: forall a. IsSyntheticEvent a => a -> Boolean
cancelable = (#) _.cancelable <<< unsafeCoerce <<< toSyntheticEvent
currentTarget :: forall a. IsSyntheticEvent a => a -> DOMEventTarget
currentTarget = (#) _.currentTarget <<< unsafeCoerce <<< toSyntheticEvent
nativeEvent :: forall a. IsSyntheticEvent a => a -> DOMEvent
nativeEvent = (#) _.nativeEvent <<< unsafeCoerce <<< toSyntheticEvent
preventDefault :: forall eff a. IsSyntheticEvent a => a -> Eff eff Unit
preventDefault = (#) _.preventDefault <<< unsafeCoerce <<< toSyntheticEvent
stopPropagation :: forall eff a. IsSyntheticEvent a => a -> Eff eff Unit
stopPropagation = (#) _.stopPropagation <<< unsafeCoerce <<< toSyntheticEvent
...etc
foreign import data SyntheticClipboardEvent :: Type
class IsSyntheticClipboardEvent a where
toSyntheticClipboardEvent :: a -> SyntheticClipboardEvent
instance isSyntheticClipboardEventSyntheticClipboardEvent :: IsSyntheticClipboardEvent SyntheticClipboardEvent where
toSyntheticClipboardEvent = id
clipboardData :: forall a. IsSyntheticClipboardEvent a => a -> DOMDataTransfer
clipboardData = (#) _.clipboardData <<< unsafeCoerce <<< toSyntheticClipboardEvent
...etc Option 2 - RecordsAnother option is to continue with how the React module is currently working, using records to define the properties of the events. However, use extensible records if events share common properties. See below for wha this could look like. type SyntheticEvent eff r
= { bubbles :: Boolean
, cancelable :: Boolean
, currentTarget :: DOMEventTarget
, defaultPrevented :: Boolean
, eventPhase :: Number
, isTrusted :: Boolean
, nativeEvent :: DOMEvent
, preventDefault :: Eff eff Unit
, isDefaultPrevented :: Eff eff Boolean
, stopPropagation :: Eff eff Unit
, isPropagationStopped :: Eff eff Boolean
, target :: DOMEventTarget
, timeStamp :: Number
, type :: String
| r
}
type SyntheticKeyboardEvent eff
= SyntheticEvent eff
( altKey :: Boolean
, charCode :: Int
, ctrlKey :: Boolean
, getModifierState :: String -> Boolean
, key :: Boolean
)
...etc I am curious to hear what people think of the two options and if one option is preferred over the other. Or if someone has another option not listed above, I am definitely open to hear about it. |
Before the next major release, I was thinking of looking into options to update the way events are represented to tackle some issues brought up regarding events: #135, #85.
One option suggested in #135 (comment) could be a good way to go for this.
Thoughts and ideas on this are welcome!
/cc @natefaubion
The text was updated successfully, but these errors were encountered: