Skip to content

fix: use Vue Router in abstract mode #238

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.

Sign up for GitHub

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
wants to merge 1 commit into from
+8 −1
Closed

fix: use Vue Router in abstract mode #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/__tests__/vue-router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,9 @@ test('setting initial route', () => {

expect(getByTestId('location-display')).toHaveTextContent('/about')
})

test('router state is not shared between tests', () => {
const {getByTestId} = render(App, {routes})

expect(getByTestId('location-display')).toHaveTextContent('/')
})
3 changes: 2 additions & 1 deletion src/render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ function render(
const VueRouter = requiredRouter.default || requiredRouter
localVue.use(VueRouter)

router = new VueRouter({routes})
router = new VueRouter({routes, mode: 'abstract'})
router.push('/')
Copy link
Contributor Author

@blimmer blimmer Jun 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This felt like the most reasonable default to use, since in abstract mode the stack is empty. In the default hash mode with a clean JSDOM environment, this would also be the behavior.

Alternatively, we could require the user to push their first route, but that would be a more breaking change from the existing behavior. I'm curious what the maintainer's stance on this is. Do we want to retain more compatibility with older versions of the library? Or would you prefer to leave the initial route stack to stay uninitialized for more declarative tests?

Copy link
Member

@afontcu afontcu Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hi! First of all, thank you for tackling this one!

I think I'll take a closer look, as even though this fixes the issue, we might have some unexpected behavior here for users who would expect a regular, history-based router (not entirely sure what are the differences with abstract mode).

Maybe we could follow a similar approach to #232, where we allow users to pass their own instance of Vuex. This would allow people to set their own router mode while also preserving backwards compatibility and consistency between vuex/router options. What do you think about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a fair point that users might expect a hash-based / history router to be used with this library. In my case, at least, having to pass the entire Vue Router would be a bit of a pain to have to do with each test, as abstract mode is the configuration I'd prefer to use throughout all my tests. So, at least initially, my thought is that some kind of global config file would be useful, for example:

// vue-testing-library.config.js
module.exports = {
  vueRouterMode: 'abstract',
};

However, it looks like other testing-library plugins don't have a global configuration file where a user could set something like this. For instance, react-testing-library has support for a custom render function (docs) where the user can set defaults.

So perhaps the way forward is to allow passing an instantiated Vue Router, but then also provide docs for a custom render function like the react-testing-library docs above? Either way, I think it also makes sense to add a point to the "FAQs" page about this, as it's a bit confusing no matter what we decide to do.

I'm happy to tackle these updates depending on the maintainer's preferences. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

For instance, react-testing-library has support for a custom render function (docs) where the user can set defaults

We can do the same thing here at Vue Testing Lib:

import VueRouter from 'vue-router'
import MyComponent from './MyComponent'
import routes from './routes'

function customRender() {
  const router = new VueRouter({ routes, mode: 'abstract' })
  router.push('/')

  return render(MyComponent, { routes: router })
}

// and then

test('foo', () => {
  const { getByText } = customRender()
})

This would allow people to set up their custom Router's mode a single time

}

if (configurationCb && typeof configurationCb === 'function') {
Expand Down