Skip to content

Commit

Permalink
Adding Wrapper suffix to EventHandler and EventHandler type names
Browse files Browse the repository at this point in the history
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
  • Loading branch information
shergin authored and facebook-github-bot committed May 29, 2018
1 parent 57e7556 commit 957ef60
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions ReactCommon/fabric/core/events/EventPrimitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,11 @@ enum class EventPriority {
Deferred = AsynchronousBatched,
};

// `InstanceHandler`, `EventTarget`, and `EventHandler` are all opaque
// raw pointers. We use `struct {} *` trick to differentiate them in compiler's
// eyes to ensure type safety.
using EventTarget = struct {} *;
using EventHandler = struct {} *;

} // namespace react
} // namespace facebook

0 comments on commit 957ef60

Please sign in to comment.