Skip to content
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

Update Vue Test Utils to 2.4.1 #830

Merged
merged 2 commits into from
Jul 25, 2023
Merged

Update Vue Test Utils to 2.4.1 #830

merged 2 commits into from
Jul 25, 2023

Conversation

matthew-white
Copy link
Member

@matthew-white matthew-white commented Jul 24, 2023

I want to update Vue to the latest version for a couple of reasons, but the latest Vue seems to require a later version of Vue Test Utils (VTU) than what we're using. I tried updating VTU to the latest, but I ran into a separate issue. This PR resolves that issue and updates VTU to the latest. Once this PR is merged, I'll update Vue itself.

The issue I ran into has to do with access to global properties. Many tests make assertions about or otherwise reference global properties, specifically $route and $router. However, it looks like when they're accessed from the vm property of the VTU wrapper, global properties are undefined. For example, in a test that makes an assertion about app.vm.$route.path, I see this error:

TypeError: Cannot read properties of undefined (reading 'path')

This issue seems to be specific to global properties. Other properties of the vm property are accessible, including properties passed to defineExpose() and properties passed to the mocks option of the VTU mount() function. Even vm.$i18n is accessible: it looks like vm.$i18n isn't a global property and instead is defined by a global mixin. However, $route and $router are global properties.

Central Frontend only defines a single global property of its own: $tcn. If I log vm.$tcn in testing, I can see that $tcn is also now undefined. So it really seems to be all global properties that are affected.

To be clear, components themselves don't seem to be throwing errors. Components appear to be able to access global properties. It's just the vm property of the VTU wrapper that no longer provides access to those properties.

Searching issues in the VTU repo, vuejs/test-utils#2008 sounds very similar. Based on that discussion, it's possible that this issue will go away once we upgrade some of our Frontend infrastructure, including by moving to Vite (#671). However, it'd be great to upgrade Vue and VTU before making the move to Vite. It's also not completely clear from the issue what the solution ended up being (though I've now asked). I did try just updating to the latest Vue (updating vue and @vue/compiler-sfc in addition to @vue/test-utils), but that didn't help.

vuejs/test-utils#1869 also came up in my search, but I don't think it's related. That issue pointed specifically to Options API components with a setup() function, but in Central Frontend, vm.$route seems to be undefined for all components. The issue also highlighted global properties whose names start with $. However, even if I rename the $tcn global property to tcn, vm.tcn is undefined.

I considered a few different workarounds for this issue. Since app.vm.$container is still accessible (since it's a VTU mock), I could have replaced all instances of vm.$route with vm.$container.router.currentRoute.value. However, I wanted to avoid changing a lot of tests, and vm.$route is nice and short. I also didn't want to change how things work in production.

Ultimately, I was able to resolve the issue by modifying our mount() function. mount() will now make $route and $router accessible by shadowing them with computed properties that reference $container. The Vue docs about global properties imply that shadowing is allowed. The values of the computed properties should be exactly the same as the global properties, but because they're computed properties on the component (injected by a global mixin), they're accessible from vm. This strategy is similar to how Vue I18n is set up, as Vue I18n is able to avoid global properties by using a global mixin.

Besides working on this issue, I did a quick search of VTU release notes for other breaking changes. I didn't see anything, and once the issue here was resolved, tests started passing again. 🎉 I also confirmed that tests continue to pass after updating to the latest Vue (but I'll do that update in a follow-up PR).

Before submitting this PR, please make sure you have:

  • run npm run test and npm run lint and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

I ran the following command:

npm update @vue/test-utils
@matthew-white matthew-white requested a review from sadiqkhoja July 24, 2023 15:30
Copy link
Contributor

@sadiqkhoja sadiqkhoja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable

@matthew-white
Copy link
Member Author

vm.$route seems to be undefined for all components.

I actually found a component for which vm.$route isn't undefined: if I use mockRouter() in the test of PageSection, I'm able to access vm.$route. That made me realize that global properties are accessible from vm for simple Options API components, but not Composition API components or Options API components with a setup() function. I've filed an issue with Vue Test Utils (vuejs/test-utils#2140), but I think we can still merge this PR as a workaround.

@matthew-white matthew-white self-assigned this Jul 25, 2023
@matthew-white matthew-white merged commit eb8849c into master Jul 25, 2023
@matthew-white matthew-white deleted the update-vtu branch July 25, 2023 19:07
@matthew-white matthew-white mentioned this pull request Jul 25, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ done
Development

Successfully merging this pull request may close these issues.

2 participants