fix: multi source update #3892
fix: multi source update #3892
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## formily_next #3892 +/- ##
================================================
+ Coverage 96.59% 96.60% +0.01%
================================================
Files 152 152
Lines 6695 6725 +30
Branches 1869 1824 -45
================================================
+ Hits 6467 6497 +30
- Misses 200 228 +28
+ Partials 28 0 -28
☔ View full report in Codecov by Sentry. |
|
这个改动量有点大，仔细说说你的思路吧 |
|
改动量还好，兼容历史版本的。 思路：调整 _boundary 的类型从 number 改为 Map<Target, Key>，用于存储更新源的信息，避免嵌套更新多个源时被忽略。target 和 key 在 runReactions 时保存到 reaction 中，执行 batchEnd 之前，保存当前的更新源，batchEnd 后有更新的，如果更新的 target 和 key 已经在 Map 中并且匹配上了，那么忽略更新（历史版本），否则说明是其他更新源，执行 tracker 重新进行依赖收集。 改动：
|
|
#3666 这个问题我感觉也能一起修，你可以构造一下测试用例看看，这是他的 reactive 用例 |
|
因为你的 _updateKey 是单个变量，并且在 runReactions 中不断替换。如果出现间接循环那这个 _boundary 的过滤就会失效。 单测例子： test('repeat execute autorun cause by deep indirect dependency', () => {
const obs: any = observable({ aa: 1, bb: 1, cc: 1 })
const fn = jest.fn()
const fn2 = jest.fn()
const fn3 = jest.fn()
autorun(() => fn((obs.aa = obs.bb + obs.cc)))
autorun(() => fn2((obs.bb = obs.aa + obs.cc)))
autorun(() => fn3((obs.cc = obs.aa + obs.bb)))
expect(fn).toBeCalledTimes(4)
expect(fn2).toBeCalledTimes(4)
expect(fn3).toBeCalledTimes(3)
})
|
|
处理了 |
|
因为你的 _updateKey 是单个变量，并且在 runReactions 中不断替换。所以如果在一个 batch 中存在多个 key 的值更新，那这个 _boundary 的过滤就会失效。 test('repeat execute autorun cause by deep indirect dependency', () => {
const obs: any = observable({ aa: 1, bb: 1, cc: 1 })
const fn = jest.fn()
const fn2 = jest.fn()
const fn3 = jest.fn()
autorun(() => fn((obs.aa = obs.bb + obs.cc)))
autorun(() => fn2((obs.bb = obs.aa + obs.cc)))
autorun(() => fn3((obs.cc = obs.aa + obs.bb)))
expect(fn).toBeCalledTimes(4)
expect(fn2).toBeCalledTimes(4)
expect(fn3).toBeCalledTimes(3)
fn.mockClear()
fn2.mockClear()
fn3.mockClear()
batch(() => {
obs.aa = 1
obs.bb = 1
obs.cc = 1
})
expect(fn).toBeCalledTimes(2)
expect(fn2).toBeCalledTimes(2)
expect(fn3).toBeCalledTimes(2)
})
|
|
这个 case 我处理一下 |
|
autorun对待递归来源时需要至多执行一次，你的方案失去了递归响应能力 test('autorun should trigger it self at most once', () => {
const obs = observable({ aa: 1, bb: 1 })
autorun(() => {
obs.aa = obs.bb + 1
obs.bb = obs.aa + 1
})
expect(obs.aa).toBe(4)
expect(obs.bb).toBe(5)
autorun(() => {
obs.aa = obs.bb + 1
obs.bb = obs.aa + 1
})
expect(obs.aa).toBe(10)
expect(obs.bb).toBe(11)
})
|
|
这个我知道，#3666 一样的问题 |
|
@hchlq 改的怎么样了呢 |
|
@janryWang 这块我还在想哈。 还没有更好的方案，目前是解决了多个源更新的问题，但是还有个问题是「递归来源时需要至多执行一次」这个问题还没有解决 |
|
@janryWang @gwsbhqt 「递归来源时需要至多执行一次」这个应该还需要讨论下，看了下 formliy 之前的实现和参考了 vue 的 reactivity，autorun 中执行的更新都是会忽略的，因此上面的 testcase 预期应该是这样的： test('multiple update should trigger only one', () => {
const obs = observable({ aa: 1, bb: 1 })
autorun(() => {
obs.aa = obs.bb + 1
obs.bb = obs.aa + 1
})
expect(obs.aa).toBe(2)
expect(obs.bb).toBe(3)
autorun(() => {
obs.aa = obs.bb + 1
obs.bb = obs.aa + 1
})
expect(obs.aa).toBe(6)
expect(obs.bb).toBe(7)
}) |
hchlq
force-pushed
the
fix/multi-source-update
branch
from
d97dfa0
to
7f859a3
Compare
|
@janryWang 难解决的原因：在 batch 中执行到 |
|
@hchlq @janryWang 这个问题现在有什么进展吗？同遇到了这个问题，而且感觉还挺容易触发的 |
Before submitting a pull request, please make sure the following is done...
masterorformily_next.npm test).npm run lint) - we've done our best to make sure these rules match our internal linting guidelines.Please do not delete the above content
What have you changed?
fix #3837