vuejs / vue-router

🚦 The official router for Vue 2
http://v3.router.vuejs.org/
MIT License
18.99k stars 5.06k forks source link

Refactoring options api to use `vue-router/composables` break basic unit tests #3801

Closed renatodeleao closed 1 year ago

renatodeleao commented 1 year ago

Heya! đź‘‹ While I was waiting for the 3.6.x release, I rolled some in-house vue-router hooks as per https://github.com/vuejs/vue-router/issues/3760#issuecomment-1191774443. This allowed refactoring some options-API components to composition-api "mode" while keeping my test suite intact.

I thought the refactor to use the official package hooks would be a simple find/replace but my test suite broke. After some digging, the reason is that the hooks implementation relies on router.currentRoute property and afterEach hook, which made the tests using the simplest $route mock as suggested in vue-test-utils docs to fail.

https://github.com/vuejs/vue-router/blob/e3273b3db905f0d162d57725fba0c459ea5bb463/src/composables/globals.js#L23-L30

Steps

  1. Giving a basic options API component

    <!-- using composition API -->
    <template>
       <div>{{ msg }}</div>
    </template>
    
    <script>
    import { useRoute } from 'vue-router/composables';
    import { computed } from 'vue';
    
    export default {
      computed: {
          msg() {
             return `My route name is ${this.$route.name}`;
          }
      }
    };
    </script>
  2. refactoring to composition api

    <!-- Refactoring using composition-api -->
    <script>
    import { useRoute } from 'vue-router/composables';
    import { computed } from 'vue';
    
    export default {
      setup() {
        const $route = useRoute();
        const msg = computed(() => {
          `My route name is ${$route.name}`;
        });
    
        return {
          msg,
        };
      },
    };
    </script>
  3. The test that worked before, now breaks.

    import { mount } from '@vue/test-utils';
    import SomeComponent from './SomeComponent.vue';
    
    const $route = {
      path: '/some/path',
    };
    
    function Wrapper() {
      return mount(SomeComponent, {
        mocks: {
          $route,
        },
      });
    }
    
    test('mount component',  () => {
      const wrapper = Wrapper();
    
      expect(wrapper.exists()).toBe(true);
      // TypeError: Cannot read properties of undefined (reading 'currentRoute')
    });

    One of The Fixes

import { mount } from '@vue/test-utils';
import SomeComponent from './SomeComponent.vue';

const $route = {
  path: '/some/path',
};

function Wrapper() {
  return mount(SomeComponent, {
    mocks: {
-      $route,
+      $router: { currentRoute: $route, afterEach: vitest.fn() },
    },
  });
}

test('mount component',  () => {
  const wrapper = Wrapper();

  expect(wrapper.exists()).toBe(true);
});

...or setting up a router with localVue as per docs... or stubbing modules jest.mock().

Expectations

Reproducing Demos

@vue/test-utils@1.3.4
vue@2.7.13
vue-router@3.6.5
`vitest` (can be any test runner)

Question

If you've went this way, I'm sure there are good reasons for it. But curious on why didn't you gt with a similar approach as vue-router@4, as in, with a reactive object derived from getCurrentInstance().proxy.$route as per https://github.com/vuejs/vue-router/pull/3763/commits/f1eb2b1dba9b5cc5011b230a3121c882d883c4c0#diff-2042e0452c0d0edd730403740acc8d4337382101087ad62a677e1e2fdae05e8fR8-R17 which kept tests working without any change — but surely break some other stuff.


Cheers and thanks for your work on the router! Feel free to close this if you think it's related with @vue/test-utils docs instead ✌️

vue-bot commented 1 year ago

Hello, thank you for taking time filling this issue!

However, we kindly ask you to use our Issue Helper when creating new issues, in order to ensure every issue provides the necessary information for us to investigate. This explains why your issue has been automatically closed by me (your robot friend!).

I hope to see your helper-created issue very soon!