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

feat: Add types for Vue 3 #180

Merged
merged 14 commits into from
Dec 3, 2020
Merged

feat: Add types for Vue 3 #180

merged 14 commits into from
Dec 3, 2020

Conversation

afontcu
Copy link
Member

@afontcu afontcu commented Nov 22, 2020

Fixes #167

Waiting on:

vuejs/test-utils#252 to get types from there.
vuejs/test-utils#254 to fix CI due to missing vue-class-component dep.

The important files of this PR are

https://github.com/testing-library/vue-testing-library/blob/vue3-types/types/index.d.ts
and
https://github.com/testing-library/vue-testing-library/blob/vue3-types/types/test.ts


Decided to rename the main file and to remove the moduleNameMapper jest option because TS wasn't getting the right types. I thought that tweaking jest+TS+whatever was worth the "import {render} from '@testing-library/vue'" statement to make tests more similar to what people do on their codebases.

@afontcu afontcu added the vue3 label Nov 24, 2020
@lmiller1990
Copy link
Collaborator

lmiller1990 commented Dec 3, 2020

Looks like this is finally unblocked. Is there anything I can do to help out and get this merged?

Looks like testing-library is not happy with a linting error in node_modules... that seems weird. vue-testing-library/node_modules/@vue/runtime-dom/dist/runtime-dom.d.ts

I am not familiar with this code base yet, or why this might be happening, my guess is this is not intended.

Edit: I ran yarn validate but got completely different errors.

@lmiller1990
Copy link
Collaborator

lmiller1990 commented Dec 3, 2020

Edit: I fixed the tests failing on my machine, but now I get something different.

(vue3-types*) $ yarn typecheck
yarn run v1.22.10
$ dtslint types
Error: Cannot find module '/Users/lachlan/.dts/typescript-installs/4.0/node_modules/typescript'
Require stack:
- /Users/lachlan/code/dump/vue-testing-library/node_modules/dtslint/bin/lint.js
- /Users/lachlan/code/dump/vue-testing-library/node_modules/dtslint/bin/index.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at testDependencies (/Users/lachlan/code/dump/vue-testing-library/node_modules/dtslint/bin/lint.js:64:16)
    at Object.<anonymous> (/Users/lachlan/code/dump/vue-testing-library/node_modules/dtslint/bin/lint.js:26:28)
    at Generator.next (<anonymous>)
    at /Users/lachlan/code/dump/vue-testing-library/node_modules/dtslint/bin/lint.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/lachlan/code/dump/vue-testing-library/node_modules/dtslint/bin/lint.js:4:12)
error Command failed with exit code 1.

🤔

Edit: had to use my local TS. I got the error CI has:

 y dtslint --localTs node_modules/typescript/lib types

Now I will work on fixing it.

@lmiller1990
Copy link
Collaborator

lmiller1990 commented Dec 3, 2020

Argh, I followed the same breadcrumbs you did and found your exact post: microsoft/dtslint#297 (comment) ... haha.

Edit: I sunk a LOT of time into figuring this out and had no luck. Tslint is always so frustrating to deal with. Can we just remove the yarn validate part of the pipeline until we figure out how to fix this? Not having types is really a deal breaker, much more so than having knowing something in node_modules is using a ts-ignore (why would a linter validate node_modules at all?). Even passing the --expectOnly flag which is supposed to only check for correctness still causes an error - it seems like dtslint is not working as expected at all?

@lmiller1990
Copy link
Collaborator

lmiller1990 commented Dec 3, 2020

Edit again: here is my findings.

  1. Importing anything from vue in the types directory breaks things. For example here.
  2. Seems there is no way to disable this - I tried everything, including hacking the source in of dtslint in node_modules.
  3. Even if you hack it to pieces to get the rid of the ts-ignore, test.ts fails. For example here
await rerender({a: 1}) // $ExpectType Promise<void> 

I am not 100% sure about this but I think this test is wrong. Since we do await, we are asserting against the resolved value of rerender, so it should be // $ExpectType void. If you want to assert a Promise, you do:

rerender({a: 1}) // $ExpectType Promise<void> 

It would be nice to solve this, but I have no idea how to proceed. Since this is still an unstable release (next), should we just get the types merged? At the moment, if you install next and are using TypeScript, the IDE will complain since there are no types.

@afontcu afontcu marked this pull request as ready for review December 3, 2020 07:11
@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #180 (f629d16) into next (7e1882a) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              next      #180   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           84        86    +2     
  Branches        26        27    +1     
=========================================
+ Hits            84        86    +2     
Impacted Files Coverage Δ
src/index.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 7e1882a...f629d16. Read the comment docs.

@afontcu afontcu changed the title Add types for Vue 3 feat: Add types for Vue 3 Dec 3, 2020
@afontcu afontcu merged commit 3ef7112 into next Dec 3, 2020
@afontcu afontcu deleted the vue3-types branch December 3, 2020 07:13
@github-actions
Copy link

github-actions bot commented Dec 3, 2020

🎉 This PR is included in version 6.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants