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

fix(reactivity): accept subtypes of collections #1864

Merged
merged 2 commits into from
Aug 17, 2020
Merged

fix(reactivity): accept subtypes of collections #1864

merged 2 commits into from
Aug 17, 2020

Conversation

unbyte
Copy link
Contributor

@unbyte unbyte commented Aug 15, 2020

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] 

close #1868

@unbyte
Copy link
Contributor Author

unbyte commented Aug 15, 2020

@Picknight I think it's a better solution than #1799

@Picknight
Copy link
Contributor

Picknight commented Aug 15, 2020

@Picknight I think it's a better solution than #1799

Yep, I think this one is better.

btw, this PR essentially replaces the type judgment basis from target.constructor to toRawType(target), I'm not sure if there is a potential problem.

@unbyte
Copy link
Contributor Author

unbyte commented Aug 15, 2020

Yep, I think this one is better.

btw, this PR essentially replaces the type judgment basis from target.constructor to toRawType(target), I'm not sure if there is a potential problem.

I've found this as a reference http://perfectionkills.com/instanceof-considered-harmful-or-how-to-write-a-robust-isarray/

@unbyte
Copy link
Contributor Author

unbyte commented Aug 16, 2020

using switch instead of map and object is based on this benchmark
https://jsben.ch/Pt9NS

@yyx990803
Copy link
Member

I like this one, thanks for iterating on this!

@yyx990803 yyx990803 merged commit d005b57 into vuejs:master Aug 17, 2020
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.

subtypes of collections like Map cause errors
3 participants