Closed afontcu closed 3 years ago
Yeah I think the idea should work out fine. It's a bit unfortunate that it'll be such a significant change. But the bright side is that render
will get to stay synchronous. As you mentioned, the external setup already exists for other plugins and other libraries.
It's significantly more comforting though to know that the global.plugins
property can be taken advantage of, though. On that note, is there any reason for not letting people pass in a plugins
property directly?
thanks for your thoughts 👍
is there any reason for not letting people pass in a
plugins
property directly
People might need to set up a variety of global stuff – plugins, directives, components, mixins, and whatnot. This is why we decided to add a global
key with several options that grouped everything that's global to the Vue app used to render the component
I agree about leaving the wrapper as thin as possible, leaving the responsibility to the end user as it is really simple to create a custom wrapper including your own setup with vuex, router and any other plugin that you would normally need in most of the projects.
Breaking compatibilty is not ideal but showing the warning and documenting how to migrate/create your own custom render
with vuex/router setup should not be a problem IMO.
TBH I always found problematic to pass store
as a parameter from VTU 2.x
as the devoloper could think that render
is expecting a real instance of the store (new Vuex.Store(storeDefinition
) instead of the storeDefinition that VTU internally uses to instanciate the store.
TL;DR:
Thanks for this amazing library! 🤟
Seems like a good compromise to me!
Here's an implementation of the RFC – https://github.com/testing-library/vue-testing-library/pull/206
Hey just a heads up, it seems the links for the VTU docs that you used in your original post don't work anymore. It might be good to update them in case anyone else comes along and wants to read.
Updated the links, thanks
Users need to initialize router/vuex. This is not the case with VTL for Vue 2, where we import the libraries when needed and attach them to the local vue instance.
FWIW, I don't see this as a "Drawback" and not sure the Alternative is good. I landed here because I was going to see if it was possible to pass our own router and store in instead of plain objects. The whole concept behind TL is to test your app as a user would use it, so not being able to actually render with the exact router and store that we're using in the app is actually a drawback IMO. So I think this is a great update, thank you!!
The RFC has received several positive comments and no hard pushbacks. I'll leave the PR open for another week and then merge it 😃
:tada: This issue has been resolved in version 6.4.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
Summary
Remove custom integrations for Vue Router and Vuex, and leave it to users to configure them as they'd do with any other plugin.
Currently, VTL for Vue 3 offers basic support for Router and Vuex, similar to VTL for Vue 2. It requires router/vuex library if the appropriate render option is provided, creates the plugin and adds it to
global.plugins
. See source.Basic example
VTL would remove support for
store
androutes
keys on render:in favor of VTU'
global.plugins
(docs).This is the suggested way to install Router/Vuex in VTU 2.x, too (see here and here).
Strictly speaking, removing integration with Vuex is not needed. However, it would be weird to only support one of the official Vue plugins, and treat the other one as a regular plugin.
Motivation
This RFC is motivated by the discussion in https://github.com/testing-library/vue-testing-library/issues/195. In short:
Vue Router is now async, so it needs special treatment when it comes to navigating (simple – just use
waitFor
orfindByXXX
queries), but also if user needs to assert something related to first navigation (Router exposes arouter.isReady()
helper). This makes an out-of-the-box management from VTL a bit… weird.Detailed design
There's not much to do on VTL. VTU 2.x offers a comfortable way to install global plugins to vue 3 app through
global.plugins
, so this is way simpler than Vue 2 (where we needed to use a local Vue instance).We're already installing anything provided through
globals.plugins
, so we should good to go. It's just a matter of docs + removingroutes
/store
options.The change would make users responsible for setting up router/vuex properly. This is the exact same thing React Testing Library users have to do (see an example for React Router and another one for Reach Router).
Drawbacks
Alternatives
render
async or exposing aisRouterReady
helper method. Both of the alternatives help us hide some of the complexity, but not all of it.Adoption strategy
We're still in beta here, so any breaking change is undesirable but expected.
People would need to:
1) Remove
routes
andstore
keys from their tests. 2) Wraproutes
withVueRouter.createRouter
(and potentially register ahistory
method. 3) Wrapstore
withVuex.createStore
. 4) Move both variables down toglobal.plugins
.We could display a warning message to make people aware of why router/vuex tests start failing:
The message would be removed before marking VTL for Vue 3 as stable.