Skip to content

fix(rerender): Do not mount component again on rerender #199

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

Merged
merged 2 commits into from
Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
23 changes: 11 additions & 12 deletions src/__tests__/rerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,24 @@ import NumberDisplay from './components/NumberDisplay'
// to ensure that the rerendered component is being updated correctly.
// That said, if you'd prefer to, for example, update the props of a rendered
// component, this function can be used to do so.
test('calling rerender remounts the component and updates the props', () => {
test('rerender re-renders the element', async () => {
const {rerender, getByTestId} = render(NumberDisplay, {
props: {number: 1},
})

expect(getByTestId('number-display')).toHaveTextContent('1')

rerender({props: {number: 3}})
await rerender({number: 3})
expect(getByTestId('number-display')).toHaveTextContent('3')

rerender({props: {number: 5}})
await rerender({number: 5})
expect(getByTestId('number-display')).toHaveTextContent('5')

// Assert that, after rerendering and updating props, the component has been remounted,
// meaning we are testing a different component instance than we rendered initially.
expect(getByTestId('instance-id')).toHaveTextContent('3')
// Notice we don't remount a different instance
expect(getByTestId('instance-id')).toHaveTextContent('1')
})

test('rerender works with composition API', () => {
test('rerender works with composition API', async () => {
const Component = defineComponent({
props: {
foo: {type: String, default: 'foo'},
Expand All @@ -43,11 +42,11 @@ test('rerender works with composition API', () => {

const {rerender, getByTestId} = render(Component)

const originalNode = getByTestId('node')
expect(originalNode).toHaveTextContent('Foo is: foo. Foobar is: foo-bar')
const getContent = () => getByTestId('node')

rerender({props: {foo: 'qux'}})
expect(getContent()).toHaveTextContent('Foo is: foo. Foobar is: foo-bar')

const newNode = getByTestId('node')
expect(newNode).toHaveTextContent('Foo is: qux. Foobar is: qux-bar')
await rerender({foo: 'qux'})

expect(getContent()).toHaveTextContent('Foo is: qux. Foobar is: qux-bar')
})
41 changes: 15 additions & 26 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
const mountedWrappers = new Set()

function render(
TestComponent,
Component,
{
store = null,
routes = null,
Expand All @@ -25,7 +25,7 @@ function render(
const baseElement = customBaseElement || customContainer || document.body
const container = customContainer || baseElement.appendChild(div)

const plugins = []
const plugins = mountOptions.global?.plugins || []

if (store) {
const {createStore} = require('vuex')
Expand All @@ -41,26 +41,20 @@ function render(
plugins.push(routerPlugin)
}

const mountComponent = (Component, newProps) => {
const wrapper = mount(
Component,
merge({
attachTo: container,
global: {plugins},
...mountOptions,
props: newProps || mountOptions.props,
}),
)

// this removes the additional "data-v-app" div node from VTU:
// https://github.com/vuejs/vue-test-utils-next/blob/master/src/mount.ts#L196-L213
unwrapNode(wrapper.parentElement)
const wrapper = mount(
Component,
merge({
attachTo: container,
global: {plugins},
...mountOptions,
}),
)

mountedWrappers.add(wrapper)
return wrapper
}
// this removes the additional "data-v-app" div node from VTU:
// https://github.com/vuejs/vue-test-utils-next/blob/master/src/mount.ts#L196-L213
unwrapNode(wrapper.parentElement)

let wrapper = mountComponent(TestComponent)
mountedWrappers.add(wrapper)

return {
container,
Expand All @@ -72,12 +66,7 @@ function render(
unmount: () => wrapper.unmount(),
html: () => wrapper.html(),
emitted: () => wrapper.emitted(),
rerender: ({props}) => {
wrapper.unmount()
mountedWrappers.delete(wrapper)

wrapper = mountComponent(TestComponent, props)
},
rerender: props => wrapper.setProps(props),
Copy link
Member Author

@afontcu afontcu Jan 8, 2021

Choose a reason for hiding this comment

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

This is mostly it. Other changes are there because we don't need a wrapper variable (and a mountComponent function) anymore.

...getQueriesForElement(baseElement),
}
}
Expand Down