Skip to content
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

filter nil event handlers to fix unexpected event target #32

Merged
merged 1 commit into from Aug 7, 2020

Conversation

chenyong
Copy link
Collaborator

@chenyong chenyong commented Aug 7, 2020

Respo resolves event by reading coord, which is attached to virtual DOM. It binds events on elements and find real virtual element in @*global-element. However, as introduced in Memof solution, virtual DOM might be reused although there coord might be different. Thus we get a wrong coord. It was not problem in the old time, but when there's an {:click nil}, it might first check there, and then use the same coord for checking parents, then we get the problem it resolving to the cached element's parents. So this fix tries to get rid of the nil events.

@chenyong chenyong requested a review from a team August 7, 2020 02:24
@chenyong
Copy link
Collaborator Author

chenyong commented Aug 7, 2020

说人话就是 Respo 处理事件的方式是在比如 div 节点上带着 #{:click} 这样的数据表示有 click 事件, 点击的时候按照节点的 coord 也就是一个 [:comp-1 :div] 这样的位置信息到 @*global-element 上反向查找事件处理的函数. 这样做挺绕的, 但是有个好处是 DOM 节点上更新减少, 而 *global-element 那边始终指向最新的节点. 本来是没问题的, 如果有个 {:click nil} 的情况, 自动就会往父节点查找了, 不影响, 只要父节点有正常的处理函数就是对的.

然而 Memof 加上以后, 在某些节点就会存在缓存的组件, 使用的是错误的 coord, 这样碰到 {:click nil} 再往上找就找到缓存节点的父节点去了, 这样就出问题了. 现在的方案就是直接把 nil 的过滤掉. 逻辑就正确了. 至于 Memof 的问题, 依然需要保证包含事件处理的节点, 不要轻易被缓存.

@soyaine soyaine merged commit f0f3c99 into master Aug 7, 2020
@soyaine soyaine deleted the fix-event-target branch August 7, 2020 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants