"Avoid mutating a prop directly" warning when updating prop

[aphofstede]

Version

1.0.0-beta.16

Reproduction link

https://codesandbox.io/s/vnv9wzp8rl

Steps to reproduce

See repro CodeSandbox.

• Create a minimal component (HelloWorld)
• Create another component (base component MobileCollapseRow) that takes two props, uses one of them and includes the minimal component
• Create the final component (ContractRow) that uses the base component and passes it the prop used above, with a computed prop as value, based on its own prop
• Now create a test that passes in a new value for the prop in the last component and observe the console.

What is expected?

Prop is updated and value changes without warnings

What is actually happening?

An warning is logged, complaining about changing a prop that isn't actually touched at all:

[Vue warn]: Avoid mutating a prop directly since the value will be overwritten whenever the parent component re-renders. Instead, use a data or computed property based on the prop's value. Prop being mutated: "actions"

found in

---> at src/components/MobileCollapseRow.vue

---

Note that the test in de CodeSandbox example does succeed, but keep an eye on the console tab.

Note that the warning complains about another prop! Something fishy is going on here..

Note: Removing the child component (HelloWorld) from the base component (MobileCollapseRow) makes the warning disappear.

Note: Removing the <a> where the prop is used from the base component makes the warning disappear.

[eddyerburgh]

Thanks for the bug report.

This is caused by a recent change to handling asynchronous updates. This should be fixed in the coming weeks.

[aphofstede]

@eddyerburgh Thanks

[cdbkr]

Just wondering if it makes sense to extend the configuration of test-utils and enable/disable the warnings. It might control the Vue.config.silent when creating the instance. Would it be a correct solution?

[eddyerburgh]

Yes I think that's the only solution at the moment.

We would need to make sure no important warnings are hidden.

Also we would need to set the config to silent in all methods that interact with the instance, like setProps and trigger.

[aphofstede]

Does that mean Vue is complaining about a non-issue here, or just that you want the option to get rid of pesky logging? :)

[eddyerburgh]

Sort of.

The problem is how we implemented synchronous updates, you can read an explanation here—#676.

[eddyerburgh]

This issue was fixed in beta.19

[ghost]

@eddyerburgh I'm still seeing this happening when using wrapper.setProps in 1.0.0-beta.25 when registering a warnHandler:

something.vue

<script>
export default {
  name: 'Something',
  props: {
    value: {
      required: true,
      type: String,
    },
  },
};
</script>

<template>
  <div>{{ value }}</div>
</template>

something_spec.js

import Vue from 'vue';
import Something from './something.vue';
import { shallowMount } from '@vue/test-utils';

describe('Something', () => {
  it('warns', done => {
    Vue.config.warnHandler = done.fail;

    const wrapper = shallowMount(Something, {
      propsData: {
        value: 'first',
      },
    });

    wrapper.setProps({
      value: 'second',
    });

    done();
  });
});

leads to:

Failed: "Avoid mutating a prop directly since the value will be overwritten whenever the parent component re-renders. Instead, use a data or computed property based on the prop's value. Prop being mutated: \"value\""

      13 |     });
      14 | 
    > 15 |     wrapper.setProps({
         |             ^
      16 |       value: 'second',
      17 |     });
      18 | 

      Error: Failed: "Avoid mutating a prop directly since the value will be overwritten whenever the parent component re-renders. Instead, use a data or computed property based on the prop's value. Prop being mutated: \"value\""
      at Env.fail (node_modules/jest-jasmine2/build/jasmine/Env.js:540:34)
      at warn (node_modules/vue/dist/vue.runtime.common.js:587:26)
      at node_modules/vue/dist/vue.runtime.common.js:3345:11
      at Object.reactiveSetter [as value] (node_modules/vue/dist/vue.runtime.common.js:1004:9)
      at node_modules/@vue/test-utils/dist/vue-test-utils.js:1836:29
          at Array.forEach (<anonymous>)
      at VueWrapper.setProps (node_modules/@vue/test-utils/dist/vue-test-utils.js:1809:21)
      at Object.setProps (spec/frontend/something_spec.js:15:13)

Is there a way to avoid this?

[ghost]

Looking at the fix in #688, the above makes sense. Is there a way to get warnings from application code but not from Vue test utils? 🤔

[ghost]

Current workaround:

  Vue.config.warnHandler = (msg, vm, trace) => {
    const currentStack = new Error().stack;
    const isInVueTestUtils = currentStack.split('\n').some(line => line.startsWith('    at VueWrapper.setProps ('));
    if (isInVueTestUtils) {
      return;
    }
    done.fail(`${msg}${trace}`);
  };

[stcruy]

Workaround in a mocha test-setup:

function fixWarnHandler() {
  Vue.config.warnHandler = (msg, vm, trace) => {
    if (msg.includes('Prop being mutated:'))  return;  // False warning: abort.
    console.log(`${msg}${trace}`);                     // Real warnings pass through.
  };
}

afterEach (() => { Vue.config.warnHandler = undefined });  // Reset every time, just in case.


it('test', () => {
  fixWarnHandler();  // <--- Add only when invalid warning is anticipated.
  foo.should.equal(bar);
});

[cdbkr]

hey @gitlab-winnie thanks for reporting this.
According to the VueJS' debug system, warnHandler gets called even if silent is set to true.
https://github.com/vuejs/vue/blob/6eaf56e28d659f985934ae3a8782f90e9d6f1775/src/core/util/debug.js#L21

I am not sure whether relying on warnHandler would be the right thing. I am wondering if test-utils would be helpful decorating it, but it would be triggered anyway.
Any suggestion?

[ghost]

@cdbkr I'm not sure if I described the problem I have clear enough, so let me rephrase it:

We want to fail our test suite for any Vue warning but of course not for correct use of Vue test utils. Would it be possible to introduce a smarter way of #631 (comment) in Vue test utils? That is, catch all warnings from Vue test utils but deliver the rest to the configured warn handler.

[cdbkr]

ok, I see your point now

[vperron]

So this is still a bug, I'm encountering it as well in a totally non-prop-updating situation :)

[ghost]

@vperron I think #1062 will fix it.

@eddyerburgh Can you confirm?

[eddyerburgh]

Yes I believe so @gitlab-winnie. I can't confirm without a reproduction, but there are many subtle issues that should be solved by that PR.

[sethidden]

What exactly does #1062 fix? I'm still getting the error with stuff like:

it('whatever', () => {
    component.vm.someProp = 'a';
    expect(component.vm.someProp).toBe('a');
  }
)

[thenoseman]

I think it would be useful to also make createLocalVue() which currently only seems to accept a Vue parameter accept something like options as a second parameter.

This could then be deepMerged after the basics are setup.

Then I could simply do const localVue = createLocalVue({ silent: true }) to override the global Vue.config.silent.

Would this be possible?

I tried

import { config } from '@vue/test-utils'
config.silent = true;

to no avail.

[Timmmm]

  Vue.config.warnHandler = (msg, vm, trace) => {
    const currentStack = new Error().stack;
    const isInVueTestUtils = currentStack.split('\n').some(line => line.startsWith('    at VueWrapper.setProps ('));
    if (isInVueTestUtils) {
      return;
    }
    done.fail(`${msg}${trace}`);
  };

I recommend not doing this. new Error().stack takes a long time. If you call setProps() a lot it will lead to test timeouts. We've also had some very weird Node freezes lately and I wouldn't be that surprised if it was because of this.

The "fix" for this issue just silences all warnings, which sucks.