Merge pull request #202 from airbnb/refactor-stopPropagation

Refactor `stopPropagation` to fix #198

packages/core/src/events.ts

@@ -13,44 +13,30 @@ const processingEventBeforeDispatch = (e: any) => {
 export class GojiEvent {
  private instanceMap = new Map<number, ElementInstance>();
 
- private stoppedPropagation = new Map<number, Map<string, number>>();
-
  public registerEventHandler(id: number, instance: ElementInstance) {
  this.instanceMap.set(id, instance);
  }
 
  public unregisterEventHandler(id: number) {
  this.instanceMap.delete(id);
- this.stoppedPropagation.delete(id);
  }
 
  public triggerEvent(e: any) {
  processingEventBeforeDispatch(e);
- const { target, currentTarget, timeStamp } = e;
-
+ const { currentTarget, timeStamp } = e;
  const id = currentTarget.dataset.gojiId;
- const sourceId = target.dataset.gojiId;
-
  const type = camelCase(`on-${(e.type || '').toLowerCase()}`);
+
  const instance = this.instanceMap.get(id);
  if (!instance) {
  return;
  }
 
- let stoppedPropagation = this.stoppedPropagation.get(sourceId);
- if (stoppedPropagation && stoppedPropagation.get(type) === timeStamp) {
- return;
- }
-
  e.stopPropagation = () => {
- if (!stoppedPropagation) {
- stoppedPropagation = new Map<string, number>();
- this.stoppedPropagation.set(sourceId, stoppedPropagation);
- }
- stoppedPropagation.set(type, timeStamp);
+ instance.stopPropagation(type, timeStamp ?? undefined);
  };
 
- instance.triggerEvent(type, e);
+ instance.triggerEvent(type, timeStamp ?? undefined, e);
  }
 }
 

packages/core/src/reconciler/__tests__/instance.test.tsx

@@ -166,4 +166,82 @@ describe('ElementInstance', () => {
  setItemsCallback([2, 1, 3]);
  expect(getTextList()).toEqual(['2', '1', '3']);
  });
+
+ test.each([[true], [false]])('stopPropagation = %s', shouldStopPropagation => {
+ const onRootTap = jest.fn();
+ const onLeafTap = jest.fn();
+ const rootRef = createRef<PublicInstance>();
+ const leafRef = createRef<PublicInstance>();
+ const App = () => (
+ <View onTap={onRootTap} ref={rootRef}>
+ <View>
+ <View>
+ <View>
+ <View>
+ <View
+ onTap={e => {
+ if (shouldStopPropagation) {
+ e.stopPropagation();
+ }
+ onLeafTap();
+ }}
+ ref={leafRef}
+ >
+ Click
+ </View>
+ </View>
+ </View>
+ </View>
+ </View>
+ </View>
+ );
+ const { getContainer } = render(<App />);
+ const rootNode = (getContainer() as { meta: ElementNodeDevelopment }).meta;
+ let leafNode: ElementNodeDevelopment;
+ for (
+ leafNode = rootNode;
+ leafNode.children?.length;
+ leafNode = leafNode.children[0] as ElementNodeDevelopment
+ ) {
+ // do nothing
+ }
+
+ const timeStamp = Date.now();
+ // trigger event on leaf
+ gojiEvents.triggerEvent({
+ type: 'tap',
+ timeStamp,
+ currentTarget: {
+ dataset: {
+ gojiId: leafRef.current!.unsafe_gojiId,
+ },
+ },
+ target: {
+ dataset: {
+ gojiId: leafRef.current!.unsafe_gojiId,
+ },
+ },
+ });
+ act(() => {});
+ expect(onLeafTap).toBeCalledTimes(1);
+ expect(onRootTap).toBeCalledTimes(0);
+ // trigger event on root
+ gojiEvents.triggerEvent({
+ type: 'tap',
+ timeStamp,
+ currentTarget: {
+ dataset: {
+ gojiId: rootRef.current!.unsafe_gojiId,
+ },
+ },
+ target: {
+ dataset: {
+ gojiId: leafRef.current!.unsafe_gojiId,
+ },
+ },
+ });
+ act(() => {});
+ expect(onLeafTap).toBeCalledTimes(1);
+ expect(onRootTap).toBeCalledTimes(shouldStopPropagation ? 0 : 1);
+ });
 });

packages/core/src/reconciler/instance.ts

@@ -102,6 +102,8 @@ export class ElementInstance extends BaseInstance {
 
  public subtreeDepth?: number;
 
+ private stoppedPropagation: Map<string, number> = new Map();
+
  public getSubtreeId(): number | undefined {
  const subtreeMaxDepth = useSubtree ? subtreeMaxDepthFromConfig : Infinity;
  // wrapped component should return its wrapper as subtree id
@@ -224,13 +226,25 @@ export class ElementInstance extends BaseInstance {
  }
  }
 
- public triggerEvent(propKey: string, data: any) {
- const listener = this.props[propKey];
+ public triggerEvent(type: string, timeStamp: number | undefined, data: any) {
+ if (timeStamp === undefined) {
+ if (process.env.NODE_ENV !== 'production') {
+ console.warn(
+ '`triggerEvent` should be called with a `timeStamp`, otherwise it will trigger all events. This might be an internal error in GojiJS',
+ );
+ }
+ return;
+ }
+ // prevent triggering if the event has been stopped by `stopPropagation`
+ if (this.stoppedPropagation.get(type) === timeStamp) {
+ return;
+ }
+ const listener = this.props[type];
  if (listener) {
  if (typeof listener !== 'function') {
  if (process.env.NODE_ENV !== 'production') {
  console.warn(
- `Expected \`${propKey}\` listener to be a function, instead got a value of \`${typeof listener}\` type.`,
+ `Expected \`${type}\` listener to be a function, instead got a value of \`${typeof listener}\` type.`,
  );
  }
  return;
@@ -240,6 +254,22 @@ export class ElementInstance extends BaseInstance {
  }
  }
 
+ public stopPropagation(type: string, timeStamp: number | undefined) {
+ if (timeStamp === undefined) {
+ if (process.env.NODE_ENV !== 'production') {
+ console.warn(
+ '`stopPropagation` should be called with a `timeStamp`, otherwise it will stop all events. This might be an internal error in GojiJS.',
+ );
+ }
+ return;
+ }
+ // traverse all ancestors and mark as stopped manually instead of using `e.target.dataset.gojiId` because of this bug:
+ // https://github.com/airbnb/goji-js/issues/198
+ for (let cursor: ElementInstance | undefined = this; cursor; cursor = cursor.parent) {
+ cursor.stoppedPropagation.set(type, timeStamp);
+ }
+ }
+
  public updateProps(newProps: InstanceProps) {
  this.previous = this.pureProps();
  this.props = removeChildrenFromProps(newProps);