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

Support for reactivity on sub-classed Arrays #6943

Closed
purtuga opened this issue Oct 27, 2017 · 4 comments
Closed

Support for reactivity on sub-classed Arrays #6943

purtuga opened this issue Oct 27, 2017 · 4 comments

Comments

@purtuga
Copy link

purtuga commented Oct 27, 2017

What problem does this feature solve?

Support sub-classed arrays.
Prevent vueJS from completely removing the subclassed array's prototype and revert it back to the native Array prototype. This in turn breaks other application code that was expecting the array to have the extended methods.

Example:

const A = class extends Array {
    constructor() {
        super();
    }
    
    push(...args) {
        console.log("push() called");
        return super.push(...args);
    }
}

const arr = new A();
arr.push(1); // console.log == "pushed() called";

const v = new Vue({
    el: mountEle,
    data: { arr }
});

arr.push(2); // no console output

(Related to #6920)

What does the proposed API look like?

No API changes.
This feature request is simply to request that an array prototype, when replaced in support of Vue's reactivity, continue to call the super method of that array instance when wrapped with VueJS's mutator code.

@posva
Copy link
Member

posva commented Oct 27, 2017

This will be supported in future versions. See #5893 (comment)

I don't think we can support arbitrary Array-like objects - the line between a actual "object" vs. "array-like" is blurry. We will stick to Array.isArray in this case. But we can and should support Array subclasses.

Also - since subclassing Array is a ES6+ feature, we will likely implement this when we re-implement the reactivity system with Proxies.

@posva posva closed this as completed Oct 27, 2017
@purtuga
Copy link
Author

purtuga commented Oct 27, 2017

@posva thanks for the reference and good to know you are looking to address this at some point in the future.

I needed to have this addressed sooner, rather than later, so I have forked the project and applied a solution to it. You can view it here if you like:

purtuga@56eba10

Test cases are passing and I will now continue on with its usage on my project. If there is any interest in taking that as a contribution, I will be happy to make it.

(BTW: one test cases was failing even before I made any changes (test: transition out-in on async component (resolve after leave complete)... Just FYI)


For anyone finding this and wanting to use the work-around until the Vue team is ready to introduce this feature: I have created a branch in my forked repo that includes the built files with my changes. You can use it by:

npm i purtuga/purtuga/vue#2.5.2-sub-class-support --save-dev

@yyx990803
Copy link
Member

Thanks @purtuga - that's an interesting approach. While for core we will probably hold on for this, there are a few things that can be improved for your fork. In particular, you are creating a fresh prototype object for every Array being observed, but technically you only need to create a wrapper prototype object once for each original prototype object. This cost can be avoided by having a Map that serves as a cache.

@purtuga
Copy link
Author

purtuga commented Oct 28, 2017

Thank @yyx990803 for taking the time to look at the fork.
That's a good point about the caching of the prototype. I normally use WeakMap for that type of stuff, but I could not any usages of it in Vue's codebase... I did find use of Map, but it was inside of src/server... Perhaps I will create a property on the original prototype and store the VueJS wrapped prototype there. Only potential issue with that approach is if the Prototype been frozen (Object.freeze)

Thanks again. Looking forward to when this feature lands on VueJS.

ashtonmeuser pushed a commit to ashtonmeuser/vue-cloudwatch-dashboard that referenced this issue Oct 10, 2018
Honestly don't know why Array subclass worked before. Refer to vuejs/vue#4049, vuejs/vue#6943, vuejs/vue#5893.
ashtonmeuser added a commit to ashtonmeuser/vue-cloudwatch-dashboard that referenced this issue Oct 10, 2018
Honestly don't know why Array subclass worked before. Refer to vuejs/vue#4049, vuejs/vue#6943, vuejs/vue#5893.
happyman125 added a commit to happyman125/Vue_Dashboard that referenced this issue Jan 28, 2020
Honestly don't know why Array subclass worked before. Refer to vuejs/vue#4049, vuejs/vue#6943, vuejs/vue#5893.
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

No branches or pull requests

3 participants