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

[PoC] VTL@next: Update to Vue 3.0 and Vue Test Utils 2.0 #137

Merged
merged 43 commits into from
Oct 31, 2020
Merged

[PoC] VTL@next: Update to Vue 3.0 and Vue Test Utils 2.0 #137

merged 43 commits into from
Oct 31, 2020

Conversation

afontcu
Copy link
Member

@afontcu afontcu commented Apr 27, 2020

This is an ongoing proof of concept to update Vue Testing Library to handle Vue 3 and Vue Test Utils 2.

Missing steps:

  • Update to Vue 3
  • Update to VTU 2 (beta)
  • Remove localVue
  • Update configs to use global config object
  • Fix issues regarding additional wrapping markup (VTU-next renders additional markup vuejs/test-utils#219)
  • Remove mountOptions.propsData as a way of providing props
  • Remove isUnmounted
  • Update Vuex
  • Update Vue-router
  • Update third-party libraries such as Vuetify, vue-i18n, vue-portal, vue-apollo… in a timely manner
  • Rethink callback parameter (the one that exposes Vue instance, which is not available anymore)
  • Rethink fireEvent.update. Is it still needed?
  • Open up RFC to further explorer breaking changes

Some identified breaking changes:

  • Snapshots are potentially different due to whitespaces.
  • Some options such as stubs are now moved to global.stubs.
  • Previous usage of vue instance as a parameter for the callback function is not replaced with global config options (for example, global.directives ). Not sure if (1) there's a better, less breaking way, or if (2) we should provide some sort of guidance (a console warning or so).
  • isUnmounted is gone.
  • updateProps is now called setProps to align with VTU.

Also: I uploaded tsconfig.json, but I'm not 100% positive we need that? edit: gone

@afontcu afontcu added the BREAKING CHANGE This change will require a major version bump label Apr 27, 2020
@afontcu afontcu changed the title [PoC] Update to Vue 3.0-beta and Vue Test Utils 2.0-beta [PoC] Update to Vue 3.0-beta and Vue Test Utils 2.0-alpha Apr 27, 2020
@cilice
Copy link
Contributor

cilice commented May 22, 2020

@afontcu What is your opinion on not going after VTU, but instead just using normal rendering of vue within jsdom? Similar to https://github.com/testing-library/react-testing-library/blob/master/src/pure.js, which doesn't depend on the react-test-renderer.

@afontcu
Copy link
Member Author

afontcu commented May 22, 2020

@afontcu What is your opinion on not going after VTU, but instead just using normal rendering of vue within jsdom? Similar to https://github.com/testing-library/react-testing-library/blob/master/src/pure.js, which doesn't depend on the react-test-renderer.

I think VTU handles a lot of stuff internally that we can benefit from, especially given that VTU-next is being rewritten (I'm on the team) and there's some cool stuff Vue Testing Library could leverage such as plugins.

@cilice
Copy link
Contributor

cilice commented May 22, 2020

@afontcu What is your opinion on not going after VTU, but instead just using normal rendering of vue within jsdom? Similar to https://github.com/testing-library/react-testing-library/blob/master/src/pure.js, which doesn't depend on the react-test-renderer.

I think VTU handles a lot of stuff internally that we can benefit from, especially given that VTU-next is being rewritten (I'm on the team) and there's some cool stuff Vue Testing Library could leverage such as plugins.

I think I understand that, I just really like the ideological idea of having same renderer the apps use for testing and consider it highly valuable.
The only thing that comes to my mind that we would really miss is the emit mixin from vtu-next, since there is no way to get emitted() otherwise, if I'm not mistaken.

@lmiller1990
Copy link
Collaborator

lmiller1990 commented May 22, 2020

If you wanted to decouple from test-utils, you could reimplement the bits you need like emitted.

That said, I don't see a huge benefit in doing so - is the VTU dependency causing problems? Any benefit to moving away from it? I see VTU as a kind of low-level lib that people can, and should, build on top of.

@cilice
Copy link
Contributor

cilice commented May 22, 2020

That said, I don't see a huge benefit in doing so - is the VTU dependency causing problems? Any benefit to moving away from it? I see VTU as a kind of low-level lib that people can, and should, build on top of.

@lmiller1990 I was under the the false impression that VTU uses some different renderer internally, but after looking at the code it all looks fine. So sorry!

I'm all good to with this approach 🙌

@lmiller1990
Copy link
Collaborator

Cool! VTU v2 (for Vue 3) is actually a tiny library, around 800 lines, and it doesn't do too much.

I am very excited to use testing-library with Vue 3

@cilice
Copy link
Contributor

cilice commented May 22, 2020

Another idea I just had:

  • updateProps is now setProps to align with VTU.

Lots of testing libraries under the umbrella provide a rerender, that more or less do what setProps would do. Maybe it's worth aligning towards other testing libraries apis and not going after VTU in this particular case?

@afontcu
Copy link
Member Author

afontcu commented May 22, 2020

Another idea I just had:

  • updateProps is now setProps to align with VTU.

Lots of testing libraries under the umbrella provide a rerender, that more or less do what setProps would do. Maybe it's worth aligning towards other testing libraries apis and not going after VTU in this particular case?

Hum, that's a good one. Not sure if "rerender" could mean more changes in the component other than updating props – what about immediately watch elements, lifecycles…? Note that this isn't bad per se – It might lead to more realistic scenarios.

The migration path would be simple, too.

@lmiller1990
Copy link
Collaborator

Should this even have setProps? That goes against the testing library philosophy, doesn't it? Something about your tests resembling user behavior.

@afontcu
Copy link
Member Author

afontcu commented May 22, 2020

Should this even have setProps? That goes against the testing library philosophy, doesn't it? Something about your tests resembling user behavior.

yeah… it was already there when I started collaborating with the library, and I didn't want to introduce a breaking change. However, as @cilice pointed out, even React Testing Lib provides a rerender method which is quite similar to setting props again.

It comes with a warning, though: "It'd probably be better if you test the component that's doing the prop updating to ensure that the props are being updated correctly"

@lmiller1990
Copy link
Collaborator

Hm, I like setProps better than rerender, personally.

Fair enough - not against keeping this API.

@cilice
Copy link
Contributor

cilice commented May 22, 2020

To document other testing libraries with rerender:

  • React:
const { rerender } = render(<NumberDisplay number={1} />)

// re-render the same component with different props
rerender(<NumberDisplay number={2} />)
  • Marko
import { render } from '@marko/testing-library'
import Greeting from './greeting.marko'

const { rerender, debug } = await render(Greeting, { name: 'World' })

// re-render the same component with different props
await rerender({ name: 'Marko' })
  • Svelte
const { container, rerender } = render(Comp, { props: { name: 'World 1' } })

expect(container.firstChild).toHaveTextContent('Hello World 1!')
rerender({ props: { name: 'World 2' } })

But I agree that it somehow feels weird, it's possible but shouldn't be prominent, just like the testing library documentation suggests.

@afontcu
Copy link
Member Author

afontcu commented Jun 16, 2020

I think I'm gonna open up an issue/RFC with proposals for Vue3-powered Vue Testing Lib

I'm planning on doing so but I'll wait until Vue 3 and VTU 2 releases. This way we can make sure we can iterate and align with them.

@afontcu afontcu changed the title [PoC] Update to Vue 3.0-beta and Vue Test Utils 2.0-alpha [PoC] Update to Vue 3.0 and Vue Test Utils 2.0 Oct 9, 2020
@@ -1,23 +1,25 @@
import {render, cleanup} from '@testing-library/vue'
import Vue from 'vue'
test.todo('check if this test still makes sense')
Copy link
Member Author

Choose a reason for hiding this comment

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

re: #142

@afontcu afontcu changed the title [PoC] Update to Vue 3.0 and Vue Test Utils 2.0 [PoC] VTL@next: Update to Vue 3.0 and Vue Test Utils 2.0 Oct 14, 2020
@codecov
Copy link

codecov bot commented Oct 18, 2020

Codecov Report

Merging #137 into next will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              next      #137   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           70        53   -17     
  Branches        13         9    -4     
=========================================
- Hits            70        53   -17     
Impacted Files Coverage Δ
src/vue-testing-library.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4da088...38f53ae. Read the comment docs.

@afontcu afontcu marked this pull request as ready for review October 31, 2020 18:51
@afontcu afontcu changed the base branch from master to next October 31, 2020 18:52
@afontcu afontcu merged commit bcf2951 into next Oct 31, 2020
@afontcu afontcu deleted the vue3 branch October 31, 2020 19:15
afontcu added a commit that referenced this pull request Oct 31, 2020
BREAKING CHANGE:
- Snapshots are potentially different due to whitespaces.
- Some options such as stubs are now moved to `global.stubs`.
- Previous usage of vue instance as a parameter for the callback function is not replaced with global `config` options (for example, `global.directives`).
- `isUnmounted` is gone.
- `updateProps` is now called setProps to align with VTU.

Some of the missing steps is to provide support to Vue Router, and also to update libraries of the ecosystem to their Vue 3 version.

This is an exciting release! 🎉  It marks the first release aiming to support Vue 3 and Vue Test Utils 2. Please [head to the PR](#137) to get more information, and feel free to open up issues and PRs to fix missing features / ports 😄
afontcu added a commit that referenced this pull request Oct 31, 2020
BREAKING CHANGE:
- Snapshots are potentially different due to whitespaces.
- Some options such as stubs are now moved to `global.stubs`.
- Previous usage of vue instance as a parameter for the callback function is not replaced with global `config` options (for example, `global.directives`).
- `isUnmounted` is gone.
- `updateProps` is now called setProps to align with VTU.

Some of the missing steps is to provide support to Vue Router, and also to update libraries of the ecosystem to their Vue 3 version.

This is an exciting release! 🎉  It marks the first release aiming to support Vue 3 and Vue Test Utils 2. Please [head to the PR](#137) to get more information, and feel free to open up issues and PRs to fix missing features / ports 😄
@ITenthusiasm
Copy link
Contributor

On the comment regarding rerendering, I think some people like the idea of "never directly touching props", so to speak, to avoid the idea of touching state or props. That seems to flow with the guidelines. Additionally, it minimizes confusion by maintaining a familiar API. I just started working with VTL and almost got lost due to the difference. So I think there's value in adding the function.

It's interesting that the RTL docs have the aforementioned warning. In his series on testing JavaScript, Kent C. Dodds advocates for using the rerender function, and iirc he's the one who brought the testing library family to life.

@afontcu
Copy link
Member Author

afontcu commented Nov 14, 2020

Hi! Yeah, it makes sense. I guess maintaining a familiar API with other testing libs make sense, as anyone coming from Vue Test Utils will still need to learn the new API. Fancy to open un a PR with the renaming? Make sure you make it point to next branch :)

@ITenthusiasm
Copy link
Contributor

I'm not as familiar with Vue as I am with React, but I can at least try to take a crack at it.

In the meantime, there's another PR resolving a bug with the current updateProps function if anyone wants to check it out. 😏

luqven added a commit to imgix/vue that referenced this pull request Apr 21, 2021
This commit updates `imgix-component.spec.ts` to use
`@testing-library/vue`'s new beta syntax. This means renaming
`propsData` to `props`. The commit also updates the Vue app syntax to
use the new Global API from vue 3. For more context on this change see
here: testing-library/vue-testing-library#137
luqven added a commit to imgix/vue that referenced this pull request Jul 1, 2021
This commit updates `imgix-component.spec.ts` to use
`@testing-library/vue`'s new beta syntax. This means renaming
`propsData` to `props`. The commit also updates the Vue app syntax to
use the new Global API from vue 3. For more context on this change see
here: testing-library/vue-testing-library#137
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This change will require a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants