Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

feat: update Vue 2 version to use @testing-library/dom v8 #284

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

Closed

Conversation

Copy link

@oskarols oskarols commented Sep 27, 2022 •
edited
Loading

Hi! 👋 I was told here that a contribution to bump @testing-library/dom to v8 would be happily received, so here I am 😄

I realize I'm opening this against the wrong branch; there doesn't seem to be one for the Vue 2 version that continues from v5.8.3. Maybe you can assist me with what to do here?

This updates the version of @testing-library/dom from v7 to v8 for Vue 2.x.

  • Since the v8 version dropped support for Node 10, I cherry-picked this change as well.
  • Using ^8.5.0 to be consistent with the Vue 3 version of the library.
  • Tested this change on a large Nuxt 2 repo and everything worked as expected.

Issues

  • There's an error when running npm run validate, unrelated to this change:
[typecheck] Error: Errors in typescript@4.9 for external dependencies:
[typecheck] ../node_modules/@types/inquirer/index.d.ts(80,76): error TS2344: Type 'T' does not satisfy the constraint 'Answers'.
[typecheck] ../node_modules/@types/inquirer/index.d.ts(84,91): error TS2344: Type 'TChoiceMap' does not satisfy the constraint 'Answers'.
[typecheck] ../node_modules/@types/inquirer/index.d.ts(84,105): error TS2344: Type 'T' does not satisfy the constraint 'Answers'.
[typecheck] ../node_modules/@types/inquirer/index.d.ts(139,43): error TS2344: Type 'T' does not satisfy the constraint 'Answers'.
[typecheck] ../node_modules/@types/inquirer/lib/objects/choices.d.ts(11,39): error TS2344: Type 'T' does not satisfy the constraint 'Answers'.
[typecheck] ../node_modules/@types/inquirer/lib/objects/choices.d.ts(11,61): error TS2344: Type 'T' does not satisfy the constraint 'Answers'.
[typecheck]
[typecheck] at /vue-testing-library/node_modules/dtslint/bin/index.js:207:19
[typecheck] at Generator.next (<anonymous>)
[typecheck] at fulfilled (/vue-testing-library/node_modules/dtslint/bin/index.js:6:58)
[typecheck] npm run typecheck --silent exited with code 1
husky > pre-commit hook failed (add --no-verify to bypass)

@oskarols oskarols changed the title (削除) feat: update v2 version to use @testing-library/dom v8 (削除ここまで) (追記) feat: update Vue 2 version to use @testing-library/dom v8 (追記ここまで) Sep 27, 2022
Copy link
Author

ping @afontcu ^ 😄

Copy link

saifahn commented Jan 20, 2023

@oskarols

I'm curious - why did you end up closing this PR?

@afontcu

Is there any possibility of this being merged and released in a new v5 version of Vue Testing Library? Our team would like to use the current option in queries to help us build more accessible components with corresponding tests 🙂

Copy link
Member

afontcu commented Jan 20, 2023

hi @saifahn! I don't have the bandwidth to work on that, but it should be possible by branching off latest v5 tag (https://github.com/testing-library/vue-testing-library/tree/v5.8.3) and submitting a PR there 👍

Copy link
Author

@saifahn Understandably this didn't get any attention and we couldn't wait, so we used the resolutions field in Yarn to override the @testing-library/dom version.

Copy link

saifahn commented Jan 21, 2023 •
edited
Loading

@afontcu

Thanks for getting back to me!
I actually created a fork branching off the v5.8.3 tag and made a commit. I was in the middle of making a PR when I noticed that I couldn't make a PR back to that tag - I can only compare the changes:

image

This makes sense because the tag simply points to a commit and is not something that can be updated. Perhaps we could make a separate branch that starts at the same point as v5.8.3 that could be used to track the v5 version and be used for releasing the v5 version?

If you agree with this approach, then we could reopen this PR and point it at that branch, seeing as the changes are the same as what I have done in my fork/branch as well. Looking forward to hearing your thoughts 🙂

@oskarols

Thanks for sharing your workaround. It looks like overrides may be what we may have to go with in npm if it comes to it.

Copy link

saifahn commented Feb 28, 2023

@afontcu

Tagging you in case this slipped under the radar. As stated above, I didn't make a PR because even after forking the v5.8.3 tag.

I wonder if you have any thoughts on my proposal? 😄

Copy link
Member

afontcu commented Mar 2, 2023

hey!

Perhaps we could make a separate branch that starts at the same point as v5.8.3 that could be used to track the v5 version and be used for releasing the v5 version?

Yeah, that sounds about right. I think it should be called 5.x, so that semantic-release is able to pick it up as a follow-up branch for v5

Copy link

saifahn commented Mar 3, 2023

Great!

Do you think you could create the branch in this repository from the 5.8.3 tag? (Seeing as I don't have those permissions 😄)

Then I can open a PR that will merge into that branch. (Or we could reopen this PR pointing there)

Thanks!

Copy link
Member

afontcu commented Mar 7, 2023 •
edited
Loading

hey @saifahn! this should be available in v5.9!

Copy link

saifahn commented Mar 8, 2023

@afontcu Thanks so much for the update 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /