Skip to content

Add dtslint setup to validate type definitions#98

Closed
kohlmannj wants to merge 3 commits intodevelopit:masterfrom
kohlmannj:dtslint
Closed

Add dtslint setup to validate type definitions#98
kohlmannj wants to merge 3 commits intodevelopit:masterfrom
kohlmannj:dtslint

Conversation

@kohlmannj
Copy link

@kohlmannj kohlmannj commented Jul 14, 2018

connects #96, #97, #99, #100, #101, #102

Description

I'm really excited to start using Unistore within a TypeScript project! However, when I installed Unistore and started using its type definitions, I noticed two errors when reviewing preact.d.ts and react.d.ts in Visual Studio Code (using TypeScript v2.9.2): #96, #97

To avoid issues with Unistore's type definitions in the future, I've added an additional test command to validate the type definitions using dtslint.

Q&A

Why "TypeScript Version: 2.2"? Evidently these type definitions as written rely upon some feature introduced in TS 2.2. dtslint recommended this addition when I tried running it without the // TypeScript Version: 2.2 comments:

dtslint Error without TypeScript Version Comments
Test with 2.1
Error: /Users/206837/Sites/kohlmannj/unistore/index.d.ts:13:19
ERROR: 13:19  expect  Compile error in typescript@2.1 but not in typescript@2.2.
Fix with a comment '// TypeScript Version: 2.2' just under the header.
Cannot find name 'object'.
ERROR: 21:41  expect  Compile error in typescript@2.1 but not in typescript@2.2.
Fix with a comment '// TypeScript Version: 2.2' just under the header.
Cannot find name 'object'.

Why are some of dtslint's rules disabled in tslint.json? My goal in this PR is to have dtslint's error output contain exactly these two errors. Future PRs might improve the type definitions by re-enabling these disabled rules.

Why are preact and react in tsconfig.json’s paths config? First, I set the path to unistore so that dtslint resolves index.d.ts for the module name ”unistore”. (dtslint recommends doing this in their docs.) That caused ”preact” and ”react” to incorrectly resolve to the two local preact.d.ts and react.d.ts files, however. Hence, I explicitly set paths to them (though I wonder if the ”react” path should be node_modules/react instead?).

Why is CI failing for this PR? CI fails right now because of the added dtslint invocation in the test script. As described above, this is symptomatic of now testing something previously untested. We can resolve this by merging fixes for both #96 and #97.

Notes

I'm not sure what's wrong with the Preact definitions in #96, so I cannot help there.

However, #97 was super easy, so I've opened #99 as well.

kohlmannj-nyt and others added 3 commits July 13, 2018 21:58
I’ve disabled a few of the standard dtslint rules for the moment. The goal of this commit is to have dstlint’s error output match just the two TypeScript errors I had encountered in Unistore v3.0.6.
@developit developit self-requested a review October 17, 2019 20:11
@kohlmannj kohlmannj closed this Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants