meteor-vue / vue-meteor-tracker

Use Meteor Tracker reactivity inside Vue components
90 stars 20 forks source link

Meteor properties over reactive? #49

Open vbgm opened 5 years ago

vbgm commented 5 years ago
<template>

  <div v-if='test1'/>

</template><script>

  export default {
    computed: {
      test1 () {
        console.log('test1')
      }
    },
    meteor: {
      $lazy: true,
      test2 () {
        console.log('test2')
      }
    }
  }

</script>

In the above example:

  1. 'test1' is triggered once - OK
  2. 'test2' is triggered once when '$lazy' is true and twice when '$lazy' is false - NOT OK

Expected behaviour: The 'test2' should not trigger at all when '$lazy' is enabled, or be triggered only once when $lazy is disabled.

Notes: 'Vue.config.meteor.freeze = true' does not fix it either.

wildhart commented 5 years ago

Same here, the meteor.property is being run at least once when the page is rendered regardless of whether it is used in the template or not, even with $lazy='true'.

What's worse, it can result in unexpected errors if the meteor.property relies on props or data which might be invalid in that situation. e.g:

<template>
    <div>
        <div v-if="user">{{user.profile.name}}</div>
        <div v-else>
            all users:
            <ul><li v-for="u in allUsers" :key="_id">{{u.profile.name}}</li></ul>
        </div>
    </div>
</template>

<script>
export default {
    props: ['user', 'query'], // we only need one of these
    meteor: {
        allUsers() {
            return Meteor.users.find(this.query, {fields: {"profile.name"}: 1);
        }
    }
}
</script>

(this is a contrived example, but I have some real world experience of this bug in more complicated components.)

Here the allUsers property isn't required if a user object is passed a prop, in which case the component might reasonably be called without the query prop at all. This would cause this.query to be undefined in allUsers(), even though allUsers isn't necessary.

My work-around is to do this instead:

export default {
    props: ['user', 'query'], // we only need one of these
    computed: {
        allUsers() { return this.$autorun(() => {
            if (!this.query) console.error('component called without any valid props');
            return this.query ? Meteor.users.find(this.query, {fields: {"profile.name"}: 1}) || [];
        })}
    }
}

Now the allUsers property isn't called unless it is actually needed (in which case the parent component should hopefully have passed the query prop instead.

vbgm commented 5 years ago

@Akryum are there any chances you could pay attention to issues with Meteor integration? To me (and hopefully to many), native Meteor (with Tracker and MiniMongo) has more advantages than Apollo.

red-meadow commented 5 years ago

@vblagomir @wildhart

Hello, guys! I decided to look under the hood and try to fix this. It turned out that we have two separated issues here: when $lazy is true and when it isn't.

The behavior of $lazy may not be a bug at all: @Akryum added it in a single commit and I guess the idea is that $lazy disables reactivity but still provides initial values.

I created two PRs: #60 and #61. I'm not sure any of them will/could be merged but I hope they will be at least reviewed.