Summary:
Removes the concept of instance handle. Instead we pass the event target
to createNode and don't pass it to subsequent clones.
The life time of the event target is managed by native (the event emitter).
It has to be released manually.
Reviewed By: shergin
Differential Revision: D8688330
fbshipit-source-id: e11b61f147ea9ca4dfb453fe07063ed06f24b7ac
Summary:
@public
After reading about move-semantic and rvalue refs I realized that we (I) definitely overuse `auto &&` (aka universal reference) construction. Even if this is harmless, does not look good and idiomatic.
Whenever I used that from a semantical point of view I always meant "I need an alias for this" which is actually "read-only reference" which is `const auto &`.
This is also fit good to our policy where "everything is const (immutable) by default".
Hence I change that to how it should be.
Reviewed By: fkgozali
Differential Revision: D8475637
fbshipit-source-id: 0a691ededa0e798db8ffa053bff0f400913ab7b8
Summary:
@public
There are some race conditions between VM objects getting deallocated and the instanceHandle held by the eventEmitter can point to deallocated memory space, causing undefined behavior like a crash.
For now, keep a strong ref to the eventTarget inside EventEmitter to avoid that scenario. This is a temporary workaround.
Reviewed By: shergin
Differential Revision: D8576785
fbshipit-source-id: 87ef36f716270ceca906b32bb86e0046ceaca19e
Summary: Calling the event emitters on the main thread seems to be problematic, so let's dispatch it via the JS thread. This requires some changes to make "eventTarget" single-use because otherwise the binding would need to synchronize the actual JS call with the act of releasing the target.
Reviewed By: shergin
Differential Revision: D8375291
fbshipit-source-id: bd2b42731176ae209f4a19c232309c163fb1c01b
Summary:
Using `EventHandlers` name was a bad idea, and I cannot tolerate it anymore.
The worst part of it is that when you have a collection of `EventHandlers` objects you cannot use plural word to describe it because `EventHandlers` is an already plural word.
And, this object is actually an event emitter, the thing on which we call events.
Reviewed By: fkgozali
Differential Revision: D8247723
fbshipit-source-id: b3303a4b9529bd6d32bb8ca0378287ebefaedda8
Summary:
This is the first attempt to implement some base part of event dispatching pipeline from end-to-end.
Even when it is working, all this is still incomplete and generally up in the air. We are still messing proper implementation of event queue, priority, and synchronization of react reconciliation process with event scheduling.
Reviewed By: fkgozali
Differential Revision: D8212271
fbshipit-source-id: 92f9427d14726441c70ffff294ac95eeb004152a
Summary:
In order to dispatch event, `EventHandlers` must also know react tag. So we have to store it inside.
We plan to illuminate this requirement (and `tag` from `EventHandlers`) eventually.
Reviewed By: fkgozali
Differential Revision: D8211685
fbshipit-source-id: 2064c0f4a7869cbf4d2c92d0349f4ee3998cb8f5
Summary:
Nothing actually changed besides type names... which actually helps me found an issue in FabricUIManager!
Now there is no a single `void *` in Fabric/C++ and JavaScript bindings. Yay!
Reviewed By: fkgozali
Differential Revision: D8191420
fbshipit-source-id: b1eb60b6bc34dd25ab200aab854ffbd7ccf5b15d
Summary:
It's maybe not so important/crucial, but this thing bothers me a lot.
We use raw opaque `EventTarget`, `InstanceHandle` and `EventHandler` pointers in application layer quite a lot and we don't have any kind of type-safety here. I believe all those opaque types should be represented as named scalar types which compiler at least can differentiate at compile time.
So I propose introducing named aliases for them which will point to particular empty `struct`s. This will allow us to tag types properly in all functions and methods and ensure that we pass right values as right arguments.
Again, they are *just aliases*, which are effectively still `void *`, no any additional logic or names are involved.
Unfortunately, those nice type names are already taken by `JSIFabricUIManager` local anonymous namespace (even if they are inside anonymous namespace we cannot use them https://stackoverflow.com/questions/3673353/anonymous-namespace-ambiguity). I think it's fair to rename them because... it's local. And we already use `Wrapper` suffix for them anyways.
Reviewed By: fkgozali
Differential Revision: D8181151
fbshipit-source-id: 9b55b43fb671a56b32a862ac54f78d528e1188ce
Summary: This implements `EventHandlers` abstract class (aka "Events Guy") which encapsulates `eventDispatcher` and `instanceHandle` (and ownership of future `eventTarget`), all of this as part of existing {ShadowNode + Props + LayoutMetrics + LocalData + Descriptor + (and now) EventHandlers} infra. (We don't plan to add anything else to this model. Ever.)
Reviewed By: fkgozali
Differential Revision: D8053351
fbshipit-source-id: 1dd9ccbcbe5a2eb284b59ea351dc8beca645e8bf
Sebastian Markbage
Valentin Shergin
Kevin Gozali