Add TypeScript type-checking to all JavaScript files and publish utility types #723
Conversation
bmish
force-pushed
the
ts-checkjs
branch
2 times, most recently
from
0577ba6
to
0e79ede
Compare
bmish
changed the title
bmish
added
enhancement
bmish
changed the title
lib/rules/no-assert-ok-find.js
Outdated
| @@ -35,6 +34,7 @@ module.exports = { | |||
| }; | |||
|
|
|||
| for (const { node } of tracker.iterateGlobalReferences(traceMap)) { | |||
| // @ts-ignore -- ESLint adds `parent` to each node. | |||
There was a problem hiding this comment.
Would it be possible to use declare module to tell it about the parent?
There was a problem hiding this comment.
So it should be yes but I'll admit it's tricky to figure out how to use declare module 'eslint-utils' to continue exporting all the same types but just tweaking the return value of one function inside one of the exported classes. If this is a bug in @types/eslint-utils, it would be better to fix it there of course, will share a PR to attempt that soon.
There was a problem hiding this comment.
Here is the fix to @types/eslint-utils: DefinitelyTyped/DefinitelyTyped#61870. Will wait for this to merge.
There was a problem hiding this comment.
Published in @types/eslint-utils 3.0.2 and updated.
lib/rules/no-test-return-value.js
Outdated
| @@ -80,6 +80,10 @@ module.exports = { | |||
| { | |||
| messageId: 'suggest', | |||
| fix(fixer) { | |||
| if (!node.range || !node.argument || !node.argument.range) { | |||
| // The if statement is just to make TypeScript happy. | |||
There was a problem hiding this comment.
In that case, would it be better to throw an error instead?
There was a problem hiding this comment.
Shouldn't matter too much but I updated to an exception.
| function isT(node, importedTranslationMethodName) { | ||
| // Example: import { t } from 'intl'; t(...); | ||
| return ( | ||
| node.type === 'CallExpression' && | ||
| isIdentifier(node.callee) && |
There was a problem hiding this comment.
Does this form of comments not support node is Identifier? That would be really helpful for narrowing down what other things the node could have.
There was a problem hiding this comment.
So yes I found out that that syntax is possible: https://github.com/open-wc/open-wc/blob/50e0ed588c67d516cb109f4106186265c239e218/packages/eslint-plugin-lit-a11y/lib/utils/aria.js#L25
But I've wanted to move away from these trivial helper functions for checking node types anyway. And removing unnecessary helper functions makes the code easier for TypeScript to understand in general.
What this does:
Benefits:
Issues I ran into:
ts-ignorethatmessageIdcan be passed asundefinedto ESLint'scontext.report()global.d.tsdist/lib/index.d.tswhich only has useful types for utils but not for rules/configs due to the usage ofrequireIndex. If we got rid ofrequireIndexand included all ofdist/in the package then the types for the rules/configs should work too. We can consider that later if there's a need.