Skip to content

Bug: Reactivity issues with @vue/compat #1665

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

Closed
Weetbix opened this issue Jul 15, 2022 · 7 comments
Closed

Bug: Reactivity issues with @vue/compat #1665

Weetbix opened this issue Jul 15, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@Weetbix
Copy link

Weetbix commented Jul 15, 2022

Describe the bug
Reactivity does not seem to work when using the vue compat build.

For example all these tests will work with vue 3, but all will fail with @vue/compat:

import {ref, onMounted, defineComponent} from 'vue';
import {it, expect} from 'vitest';
import {mount} from '@vue/test-utils';

function sleep(ms) {
    return new Promise(resolve => setTimeout(resolve, ms));
}

const TestAsync = defineComponent({
    template: '<div><div>{{ mountText }}</div><div>{{ asyncText }}</div></div>',

    props: {
        done: Function,
    },
    setup({ done }) {
        const mountText = ref();
        const asyncText = ref();

        onMounted(() => {
            mountText.value = 'mounted';
        });

        sleep(0).then(() => {
            asyncText.value = 'async';
            done?.();
        });

        return {
            mountText,
            asyncText,
        };
    },
});

it('should show onMount text', () => new Promise(done => {
    const wrapper = mount(TestAsync);
    wrapper.vm.$nextTick(() => {
        expect(wrapper.html()).toMatch('mounted')
        done();
    });
}));

it('should show async text', async () => {
    let renderedAsyncResolve;
    const renderedAsync = new Promise(resolve => renderedAsyncResolve = resolve);

    const wrapper = mount(TestAsync, {
        propsData: { done: renderedAsyncResolve }
    });

    await renderedAsync;
    expect(wrapper.html()).toMatch('async');
});

it('should show async text with nextTick', () => new Promise(async (done) => {
    let renderedAsyncResolve;
    const renderedAsync = new Promise(resolve => renderedAsyncResolve = resolve);

    const wrapper = mount(TestAsync, {
        propsData: { done: renderedAsyncResolve }
    });

    await renderedAsync;
    wrapper.vm.$nextTick(() => {
        expect(wrapper.html()).toMatch('async');
        done();
    });
}));

To Reproduce
Check out this repo on the jh/vue-utils-version branch: https://github.com/Weetbix/vue-compat-composition-api-bug-repo/blob/jh/vue-utils-version/test.spec.js

Run yarn test:vue3 to run in vue 3 and notice all tests pass.

Run yarn test:vue-compat to run with the vue compat alias, notice all tests fail.

Expected behavior
All tests should pass when running in compat mode. The templates/output should update to reflect the new values.

Related information:

  • @vue/test-utils version: 2.0.2
  • Vue version: 3.2.37
  • node version: 16.13.2
  • npm (or yarn) version: yarn 1.22.19

Additional context

Originally raised in vue testing library: testing-library/vue-testing-library#275
Similar issues in vue 2.7 but seems to be caused by vue-jest: vuejs/vue-test-utils#1983

@Weetbix Weetbix added the bug Something isn't working label Jul 15, 2022
@lmiller1990
Copy link
Member

Curious, what if you try using flushPromises?

import { flushPromises } from '@vue/test-utils'


    const wrapper = mount(TestAsync, {
        propsData: { done: renderedAsyncResolve }
    });

    await renderedAsync;
    await flushPromises()
    wrapper.vm.$nextTick(() => {
        expect(wrapper.html()).toMatch('async');
        done();
    });

Is this just a race condition?

@Weetbix
Copy link
Author

Weetbix commented Jul 21, 2022

Hey @lmiller1990 :)

I added a test with flush promises and got the same results (works with vue 3, doesnt work with compat alias):

it('with flushPromises', () => new Promise(async (done) => {
    let renderedAsyncResolve;
    const renderedAsync = new Promise(resolve => renderedAsyncResolve = resolve);

    const wrapper = mount(TestAsync, {
        propsData: { done: renderedAsyncResolve }
    });

    await renderedAsync;
    await flushPromises();
    wrapper.vm.$nextTick(() => {
        expect(wrapper.html()).toMatch('async');
        done();
    });
}));

Weetbix/vue-compat-composition-api-bug-repo@f0282f9

I think checking for the race condition was my original intention behind the done prop and awaiting renderedAsync;

@Weetbix
Copy link
Author

Weetbix commented Aug 8, 2022

@lmiller1990 any news on this? It seems that this issue makes @vue/compat not a viable upgrade path because any components written with the composition api (previously using @vue/composition-api) will not pass tests. I imagine this would affect a lot of people trying to upgrade.

Is it possibly the same issue as this? vuejs/vue-test-utils#1982 (comment)

I'm not sure, because to use compat we have to set the vue alias, so I don't think there could be two imports resolving to different distributions..

Let me know if there is anything I can do to help! 🙏

@lmiller1990
Copy link
Member

lmiller1990 commented Aug 11, 2022

I tried to reproduce in this code base so we could start working on a fix, but they all seem to pass, even in compat mode: https://github.com/vuejs/test-utils/compare/issue-1665?expand=1.

I'll debug a bit more. I'm trying to reproduce with Jest to see if this is Vitest specific.

@lmiller1990
Copy link
Member

lmiller1990 commented Aug 11, 2022

Interesting. I did cd node_modules/@vue/test-utils/dist and modified vue-test-utils.cjs.js to do require('@vue/compat) instead of require('vue') and it actually works as expected, albeit some warnings.

So... doesn't this mean the alias is not working as expected? I cannot help but think this is outside of Test Utils. We cannot change the import dynamically here - this would be up to the bundler or runner or whatever is executing the code.

I'd be curious if Jest has the same problem. I tried running your reproduction with Jest but couldn't get it working (moduleNameMapper should work like alias but it wouldn't work for some reason).

$ node index.cjs  
[Vue warn]: (deprecation RENDER_FUNCTION) Vue 3's render function API has changed. You can opt-in to the new API with:

  configureCompat({ RENDER_FUNCTION: false })

  (This can also be done per-component via the "compatConfig" option.)
  Details: https://v3-migration.vuejs.org/breaking-changes/render-function-api.html 
  at <VTUROOT>
[Vue warn]: (deprecation CONFIG_WHITESPACE) Vue 3 compiler's whitespace option will default to "condense" instead of "preserve". To suppress this warning, provide an explicit value for `config.compilerOptions.whitespace`. 
  at <Anonymous ref="VTU_COMPONENT" > 
  at <VTUROOT>
<div>
  <div>mounted</div>
  <div>async</div>
</div>

Run with node index.cjs.

package.json

{
  "dependencies": {
    "@vue/compat": "^3.2.37",
    "@vue/test-utils": "^2.0.2",
    "global-jsdom": "8.5.0",
    "jsdom": "20.0.0",
    "vue": "^3.2.37"
  }
}

index.cjs

require('global-jsdom/register')
const pkg = require('@vue/compat')
const {ref, onMounted, defineComponent, configureCompat} = pkg

configureCompat({ MODE: 2 })

const {mount, flushPromises} = require('@vue/test-utils');

function sleep(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

const TestAsync = defineComponent({
  template: '<div><div>{{ mountText }}</div><div>{{ asyncText }}</div></div>',

  props: ['done'],

  setup({ done }) {
    const mountText = ref();
    const asyncText = ref();

    onMounted(() => {
      mountText.value = 'mounted';
    });

    sleep(0).then(() => {
      asyncText.value = 'async';
      done?.();
    });

    return {
      mountText,
      asyncText,
    };
  },
});

function main () {
  return new Promise(async resolve => {
    const wrapper = mount(TestAsync, { props: resolve });
    await flushPromises();

    setTimeout(async () => {
      await flushPromises();
      console.log(wrapper.html())
    }, 100)
  })
}

main()

@lmiller1990
Copy link
Member

Edit: got it work with Jest as well, I configured moduleNameMapper in package.json under jest. https://github.com/lmiller1990/jest-vtu-compat-error

cc @Weetbix - this seems like a Vite bug?

@cexbrayat
Copy link
Member

I'll close this, as the upstream issue in Vitest mentions the proper solution (this was not an issue in VTU)

See vitest-dev/vitest#1886

@cexbrayat cexbrayat closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants