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

Reactivity: iteration of map doesn't track reads #709

Closed
jods4 opened this issue Feb 9, 2020 · 0 comments
Closed

Reactivity: iteration of map doesn't track reads #709

jods4 opened this issue Feb 9, 2020 · 0 comments

Comments

@jods4
Copy link
Contributor

jods4 commented Feb 9, 2020

Version

3.0.0-alpha.4

Reproduction link

https://jsfiddle.net/zekxpsh7/1/

Steps to reproduce

The minimal reproduction should display 'good' twice, it displays 'bad'.

For the iteration bug, here's a modified test case, from Map.spec.js

    it('should observe for of iteration', () => {
      let dummy
      const map = reactive(new Map())
      effect(() => {
        dummy = 0
        // eslint-disable-next-line no-unused-vars
        for (let [key, num] of map) {
          key
          dummy  = num
        }
      })

      expect(dummy).toBe(0)
      map.set('key1', 3)
      expect(dummy).toBe(3)
      map.set('key2', 2)
      expect(dummy).toBe(5)
      map.set('key1', 4)     // added
      expect(dummy).toBe(6)  // added
      map.delete('key1')
      expect(dummy).toBe(2)
      map.clear()
      expect(dummy).toBe(0)
    })

This modified test fails.

A similar change can be made to it 'should observe values iteration'.

What is expected?

Iterating a Map, Map.entries or Map.values should count as taking a dependency on the values inside the map.

Changing one value (without adding nor removing a key) should run effects again.

What is actually happening?

Iterating a Map only tracks addition/removal. Modifications of map contents don't run effect, resulting in unexpected, stale values.


The culprit is here:
https://github.com/vuejs/vue-next/blob/master/packages/reactivity/src/collectionHandlers.ts#L168
When enumerating entries or values on a Map, next should call track on the key before returning the results, as it's effectively a read on the entry value.

Funnily enough, Vue is aware of the problem and chooses to work-around it rather than fixing it for everyone, see:
https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/apiWatch.ts#L264-L265

ShenQingchuan added a commit to ShenQingchuan/vue-next that referenced this issue Feb 11, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Nov 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant