vue-mc icon indicating copy to clipboard operation
vue-mc copied to clipboard

Multiple layers of filtering results in infinite update loop

Open Dylan-Chapman opened this issue 6 years ago • 6 comments

My setup is roughly:

import { Trips } from "models";

export default {
    data: () => ( {
        date: "2018-06-15",
        trips: new Trips()
    } ),

    async created() {
        await this.trips.fetch();
    },

    computed: {
        selectedDateTrips() {
            return this.trips.filter( trip => trip.deliveryDate === this.date);
        },

        numCompletedTrips() {
            return this.selectedDateTrips.filter( trip => trip.status === "Completed" ).length;
        },

        numScheduledTrips() {
            return this.selectedDateTrips.filter( trip => trip.status !== "Completed" ).length;
        }
    },

    render() {
        return <p>{this.numCompletedTrips} completed trips, {this.numScheduledTrips} scheduled trips</p>;
    }
};

This gives the Vue error: "[Vue warn]: You may have an infinite update loop in a component render function."

This seems to happen whenever I'm doing two layers of filtering on an initial collection (this.trips -> this.selectedDateTrips -> completed/scheduled trip count).

This example may seem a little contrived (why don't I pass date to my api call?), but my actual setup is a bit more complicated, but essentially boils down to this.

Is there a way to avoid this?

Dylan-Chapman avatar Jun 15 '18 17:06 Dylan-Chapman

Just wanted to update this.. it looks like essentially no collection methods work on a filtered collection. Other methods I've ran into are sum, count, where, map. My guess would be that it applies to every single collection method. I'm not sure a way to get around this problem.

Dylan-Chapman avatar Jun 27 '18 18:06 Dylan-Chapman

@Dylan-Chapman looking at your example, this is something that really should be working as you'd expect it to. It's not an edge case, but a fairly simple scenario that vue-mc must support. It's currently quite difficult to test this because the tests don't render anything - maybe this a good time to introduce actual rendering and reactivity tests, which should catch cases like these.

I'll see what I can do here, sorry it has taken this long to get back to you on this.

rtheunissen avatar Jul 03 '18 13:07 rtheunissen

@rtheunissen

So, it may or may not be related, but the latest version of Vue (v2.5.17-beta.0) included some changes to computed properties. Filtering no longer works at all inside of computed properties (as far as I can tell).

I've been digging through the code a bit, and I think this is happening because when a collection is filtered, model.registerCollection() is called for each model, which modifies the model, which re-triggers the computed filter, etc.

Any thoughts?

Dylan-Chapman avatar Jul 18 '18 22:07 Dylan-Chapman

@Dylan-Chapman that's a good starting point, it does modify the model but I'm not sure why that would cause an infinite update loop. It's a shame that we don't have any rendering tests yet, they're all just unit tests, which makes it tricky to test this. We don't have the resources to implement that right now, but it seems like filter is pretty broken anyway. We're not copying any state or attributes. What I'd like to do is make some changes to the way filter works and spend some time over the weekend setting up a rendering test suite to fix this.

rtheunissen avatar Jul 30 '18 22:07 rtheunissen

Hi! Some error...

It`s happend only whin using component twice in one time. When I comment one of book-card all is fine.

Sorry for my english!

<template>
<book-card v-for="book in waitPayBooks.models" :key="book.id" :book="book"/>
<book-card v-for="book in confirmedBooks.models" :key="book.id" :book="book"/>
</template

computed:{
           waitPayBooks:function () {
               return this.cart.books.filter((book)=>book.isWaitPay);
           },
           confirmedBooks:function () {
               return this.cart.books.filter((book)=>book.isConfirmed);
           },
       },

mrFANRA avatar Sep 13 '19 14:09 mrFANRA

@rtheunissen

So, it may or may not be related, but the latest version of Vue (v2.5.17-beta.0) included some changes to computed properties. Filtering no longer works at all inside of computed properties (as far as I can tell).

I've been digging through the code a bit, and I think this is happening because when a collection is filtered, model.registerCollection() is called for each model, which modifies the model, which re-triggers the computed filter, etc.

Any thoughts?

It does look like a race condition - but maybe it's coming from the magic behind Vue's computed property caching that determines the reference to selectedDateTrips within numCompletedTrips (or numScheduledTrips).

I'm not entirely sure how the Vue watcher is determining what a "change" is for its caching mechanism, but seeing as how the computed result of selectedDateTrips should be a different, cloned Collection object every time its getter is invoked, it might be invalidating the reference within numCompletedTrips and numScheduledTrips, causing them to be re-evaluated.

robliota-tfa avatar Nov 05 '19 19:11 robliota-tfa