airbnb / goji-js

React ❤️ Mini Program
https://goji.js.org
MIT License
224 stars 27 forks source link

Missing `e.target.dataset` in event handler when crossing custom component #198

Closed malash closed 1 year ago

malash commented 1 year ago

Description

Consider this DOM tree:

<view bindtap="onClick" data-text="outside">
  <view data-text="inside">Click me</view>
</view>

When clicking on the text Click me, the onClick should be triggered with an event payload that e.target should be the inner element and the e.currentTarget should be the outer element, which means:

But if we move the inner element into a custom component:

<view bindtap="onClick" data-text="outside">
  <comp />
</view>
<!-- comp.wxml -->
<view data-text="inside">Click me</view>

The e.target became unavailable, and the dataset would always be empty.

Reproduct link

Wechat: https://developers.weixin.qq.com/s/8SSh2Dmk79GI

QQ: missing-target-id-qq.zip

Reproduct steps

  1. Open demo in WeChat/QQ dev tool.
  2. Click each line.
  3. You could find the e.target.dataset outside custom component is empty.

Impact

GojiJS uses a hacky way to implement stopPropagation: https://github.com/airbnb/goji-js/blob/8542d995892c30f02fe56967581942dd7f3d79fe/packages/core/src/events.ts#L31-L51

It checks the data-goji-id and event timestamp on both target and currentTarget to detect whether the event should be stopped or not. For example, the inner element is clicked and calls stopPropagation, GojiJS would mark this data-goji-id as stopped, then the click handler of outer element trigger and GojiJS find the target.dataset.gojiId was marked and should not run callback anymore.

In this case, the issue described above causes the stopPropagation check to fail.

Temporary solution

We have to refactor the stopPropagation without event dataset dependency. A possible way is that if stopPropagation was called, GojiJS marks clicked element and its ancestors in the virtual dom tree as stoped, the following event on marked elements should be ignored.

Platforms