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

Hotfix/observe(Array) (trouble if use extends Array) #9136

Closed
wants to merge 3 commits into from

Conversation

cybermerlin
Copy link

same PR #9135

@posva
Copy link
Member

posva commented Dec 3, 2018

What is this? What bug are you trying to fix? Can you provide a repro of the bug?
Also keep in mind all tests must pass and new tests must be added

@cybermerlin
Copy link
Author

cybermerlin commented Dec 3, 2018

something like this:

class Some extends Array {
  add(v) { this.push(v) }
}

@Component
class CompoSuper extends Vue {
  @Prop(Array) items
  created(){ this.items.add(232) }  // << exception: method .add() is undefined
}

@cybermerlin
Copy link
Author

test trouble with:


Unarchiving cache...
tar: root/project/vue/node_modules: Cannot mkdir: Permission denied```

O.o

@Justineo
Copy link
Member

Justineo commented Dec 3, 2018

If it's a bug, do you mind providing a runnable reproduction first? We are not sure what problem you are going to solve.

@cybermerlin
Copy link
Author

@Justineo look pls in #9136 (comment)

@cybermerlin
Copy link
Author

cybermerlin commented Dec 3, 2018

#6920
#8963
#5893

@posva
Copy link
Member

posva commented Dec 3, 2018

I feel like you have rushed this pr. If it's implementing that feature it would require proper testing of the actual feature request. Opening a pr with just Hotfix as a title is so
Confusing for us maintainers...

@cybermerlin cybermerlin changed the title Hotfix Hotfix/observe(Array) (trouble if use extends Array) Dec 3, 2018
@cybermerlin
Copy link
Author

@posva u r right.
n look PR #9135

@posva
Copy link
Member

posva commented Dec 3, 2018

Okay, closing this, it's not against the right branch anyway

@posva posva closed this Dec 3, 2018
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.

3 participants