Props of component stubs are not reactive

[UrsMetz]

Subject of the issue

When stubbing inner components in a test (e.g. using shallow mounting) the props of the stubs are not reactive (where as the props of a non-stub inner component are).
One example is a simple component with that passes data to an inner component and that data changes e.g. because a button is clicked, cf. example below.

Steps to reproduce

The following unit test (using jest and Vue 3) shows the difference when using stubs and real components:

import {mount} from "@vue/test-utils";
import {defineComponent} from 'vue';

describe("Props updates of inner components", () => {
    const InnerComponent = defineComponent({
        name: "inner",
        props: {
            text: {
                type: String,
                required: true
            }
        },
        template: "<div>text</div>"
    });
    const OuterComponent = defineComponent({
        name: "outer",
        components: {InnerComponent},
        data() {
            return {
                text: 'initial'
            }
        },
        template: "<div><InnerComponent :text='text'></InnerComponent><button @click='changeText'></button></div>",
        methods: {
            changeText() {
                this.text = "new text";
            }
        }
    });

    it('non-shallow mount', async () => {
        const wrapper = mount(OuterComponent, {
            shallow: false
        });

        const innerComponent = wrapper.getComponent(InnerComponent);
        await wrapper.find('button').trigger('click');
        expect(innerComponent.props()).toHaveProperty('text', 'new text');
    });

    it('shallow mount', async () => {
        const wrapper = mount(OuterComponent, {
            shallow: true
        });

        const innerComponent = wrapper.getComponent(InnerComponent);
        await wrapper.find('button').trigger('click');
        expect(innerComponent.props()).toHaveProperty('text', 'new text');
    });
});

edit:
Moving the line

const innerComponent = wrapper.getComponent(InnerComponent);

after the button click makes both test run green.
end of edit

I can also provide a repo with the whole setup but it is basically a Vue 3 app created using the CLI (version 4.5.11).

Expected behaviour

The props of the stubbed component should also be updated so that the expectation is true and both tests succeed.

Actual behaviour

One the non-shallow test case succeeds the shallow one runs red.

Possible Solution

I'd say this is a regression because with Vue 2 and "old" vue-test-utils both test would run green. So the reactive behavior of props of stubs should be resemble that of real inner components.

[lmiller1990]

Seems like a bug... I wonder why the stubbed components behave differently. As far as I can tell we pass the props to them in the same manner as a regular component - that is located here.

[UrsMetz]

One thing I forgot to mention in case that was not obvious:
when one moves the line

const innerComponent = wrapper.getComponent(InnerComponent);

after the button click both tests are green. So the stubs only present the state of their props at the moment there are obtained from the wrapper.

[lmiller1990]

Oh, I see.

I think it's likely best to get a new copy of the stub each time, anyway - this sounds like a practice we should be encouraging.

It would be nice to investigate how we can fix this, I sort of suspect getComponent is not returning a reference but a copy of the component at that point?

[cexbrayat]

Yeah it might be something like that.

@UrsMetz would you want to try to fix it and open a PR?

Starting by adding a reproduction just like the one in your issue, and maybe trying to figure out what's going on here. I suspect there is something in https://github.com/vuejs/vue-test-utils-next/blob/master/src/utils/find.ts (maybe the array spreading?) that causes this.

Let us know if you'd like to give it a try!

[UrsMetz]

@cexbrayat I think I will find some time this weekend to have a look at it :-).
@lmiller1990 I think I have used the fact that with Vue2 / vue-test-utils@1 the props were updated on stubs quite heavily and I guess a lot other also did so making this work in Vue 3 / vue-test-utils@2 would be very useful.

[lmiller1990]

It would be useful for sure. We should do this. I mean, at least we have a work around for now.

[UrsMetz]

I had a look at it:
My guess is that during a re-render of the component tree always a new stub instance is created. At least that's what it looks like when I debug my tests:
Every time a render cycle is performed line https://github.com/vuejs/vue-test-utils-next/blob/master/src/stubs.ts#L173 of the method stubComponents is reached which calls createStub. My understanding is that this creates a new stub component. The result is that the old component wrapper is not updated.
I could "fix" this by changing the code like that:

if (stub === true || shallow) {
  const propsDeclaration = type?.props || {}
  const newStub = createStub({ name, propsDeclaration, props })
  stubs[newStub.name] = newStub
  return [
    newStub,
    props,
    children,
    patchFlag,
    dynamicProps
  ]
}

The parameter stubs that is modified is global.stubs from the mount options.
Adding the stub to stubs makes resolveComponentStubByName (found some lines above in stubComponents) find our stub on a re-render.
That fix seems a very hacky to my but honestly I don't know how a clean solution would look like. I guess maybe the way the stubs are managed would need to change a bit?

edit 2021-02-21: Fixed two typos in the last paragraph.

[lmiller1990]

That solution seems like it might lead to problems leaking cached stubs between tests (maybe) or lead to other unexpected side-effects.

I am not clear on why stubComponents is called every render cycle - this does not seem correct.

Can you try adding a reproduction test, then adding your fix, and see if it breaks anything? Test Utils has a lot of hacks since we are doing things your normally cannot (and should not) do, so I don't worry too much if it feels a bit bad - as long as we have tests and comments explaining why things are the way they are, I think it's fine. I'd definitely recommend adding a link to this issue in both the test(s) you add and whatever else you modify inside of src to make this work.

There might be some benefit in using a WeakMap instead of an object to store the stubs, that would (hopefully) get cleaned up so the stubs object does not grow infinitely huge.

[UrsMetz]

@lmiller1990 I think during my debugging stubComponents wasn't called for every render cycle but rather the callback that transformVNodeArgs gets passed (but I have to double check that again).
With "reproduction test" you mean like the one I included in the issue description? I have added such a test and ran all the tests with my "fix" enabled and the passed.

[lmiller1990]

Thanks for the reproduction test!

If you want to go ahead and make a PR with your fix and test, we can review it and see if there is any obvious problems. If not, we can just go with it (if a bug does emerge in the future, we can deal with it then ... you don't know what you don't know).

[UrsMetz]

I hope I will find the time to prepare a PR this week (properly on the weekend) and will come back to you.

[UrsMetz]

I've created the PR #453 to address this issue.

[lmiller1990]

Nice job - the tests look thorough. I can review this soon. Thanks for the work!

[lmiller1990]

Merged #453