fix(reactivity): process subtypes of collections properly

[unbyte]

Fixed usage scenarios using subtypes of collections:

  class CustomMap extends Map {
    get(key) {
      console.log(key)
      return super.get(key)
    }
  }

  class SMap extends CustomMap {
    set(key, value) {
      console.log(key, value) // or other actions
      return super.set(key, value)
    }
  }

  const a = reactive(new SMap())

  a.set('a', 'ok')

// Error:  TypeError: Method Map.prototype.set called on incompatible receiver [object Object] 

[Picknight]

Same as #1490

[unbyte]

Same as #1490

That implementation introduces unnecessary impact of the runtime performance. But time loss caused by my implementation can be acceptable

I have run a simple test (node v12.18.0 on win10 using console.time()):

• the average time of my implementation is 0.007ms
• the average time of that implementation is 0.089ms
• as a contrast, const str = ` a ${var} ${anotherVar} ` takes 0.005ms

[Picknight]

Can you please post the code for the performance test, I am just curious :)

[unbyte]

Can you please post the code for the performance test, I am just curious :)

To prevent the optimization from engine affecting the test, I ran tests manually in sections (so I call it simple test).

But for show, I write this simple test, and the result is that my running time is less than half of that

const collectionTypes = new Set([Set, Map, WeakMap, WeakSet])

class CustomMap extends Map {
}

class CustomSet extends Set {
}

class CustomWeakMap extends WeakMap {
}

class CustomWeakSet extends WeakSet {
}

const testCases = new Set()

for (let i = 0; i < 1000; ++i) {
  switch (Math.floor(Math.random() * 5)) {
    case 0:
      testCases.add(new CustomMap())
      break
    case 1:
      testCases.add(new CustomSet())
      break
    case 2:
      testCases.add(new CustomWeakMap())
      break
    case 3:
      testCases.add(new CustomWeakSet())
      break
    default:
      testCases.add({})
  }
}

console.time('a')

testCases.forEach(testCase => {
  [...collectionTypes].some(c => testCase instanceof c)
})

console.timeEnd('a')

let hasB = false

console.time('b')

testCases.forEach(testCase => {
  for (const collectionType of collectionTypes) {
    if (testCase instanceof collectionType) {
      hasB = true
      break
    }
  }
})

console.timeEnd('b')


console.time('c')

testCases.forEach(testCase => {
})

console.timeEnd('c')

[Picknight]

I changed the number of for loops to 10000, below are the results of several tests(environment: MacBook Pro 16-inch, Chrome 84.0.4147.105):

a: 4.263916015625ms
b: 3.35009765625ms
c: 0.240966796875ms

a: 2.878173828125ms
b: 3.525146484375ms
c: 0.202880859375ms

a: 4.864013671875ms
b: 3.117919921875ms
c: 0.249755859375ms

It seems that the performance gap between the two implementations is not big.

[unbyte]

I changed the number of for loops to 10000, below are the results of several tests(environment: MacBook Pro 16-inch, Chrome 84.0.4147.105):

a: 4.263916015625ms
b: 3.35009765625ms
c: 0.240966796875ms

a: 2.878173828125ms
b: 3.525146484375ms
c: 0.202880859375ms

a: 4.864013671875ms
b: 3.117919921875ms
c: 0.249755859375ms

It seems that the performance gap between the two implementations is not big.

Optimization works well, but I don't think such scenario is realistic.

or is it possible to use a method of good performance but dirty that marking collection types in prototypes? I've just pushed.

[Picknight]

Yep. This is a implementation with good performance but will pollute the prototype. I am not sure this is a good solution. 🧐

[unbyte]

I think it's a trade-off: either restrict usage, or lose some performance, or fail to keep things neat. There needs to be a decision to solve the problem. After all, there is no guarantee that no subtypes of collection used in a project or imported third-party libraries.
btw, I think solution like #1490 (comment)_ is inconvenient to use because the existence of black boxes - third-party libraries.