test(eslint-plugin): render snapshots of ESLint output for each code example #8497
Conversation
|
Thanks for the PR, @auvred! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8497 +/- ##
==========================================
+ Coverage 88.02% 88.08% +0.05%
==========================================
Files 405 405
Lines 14089 14089
Branches 4125 4125
==========================================
+ Hits 12402 12410 +8
+ Misses 1382 1376 -6
+ Partials 305 303 -2
Flags with carried forward coverage won't be shown. Click here to find out more. |
packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-readonly-parameter-types.shot
Show resolved
Hide resolved
packages/eslint-plugin/docs/rules/explicit-module-boundary-types.md
Outdated
Show resolved
Hide resolved
| #### ❌ Incorrect for `requireNullish: true` | ||
|
|
||
| ```ts option='{ "requireNullish": true }' | ||
| ```ts option='{ "requireNullish": true }' skipValidation | ||
| declare const thing1: string | null; | ||
| thing1 && thing1.toString(); | ||
| ``` |
There was a problem hiding this comment.
This is a bug in the rule: #8487. For now I'll just add skipValidation...
packages/eslint-plugin/docs/rules/explicit-module-boundary-types.md
Outdated
Show resolved
Hide resolved
…es.md Co-authored-by: rubiesonthesky <2591240+rubiesonthesky@users.noreply.github.com>
packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md
Outdated
Show resolved
Hide resolved
|
🙌 unblocked now, I think! |
| beforeAll(async () => { | ||
| // dynamic import('...') is transpiled to the require('...') call, | ||
| // but all modules imported below are ESM only, so we cannot require() them | ||
| // eslint-disable-next-line @typescript-eslint/no-implied-eval | ||
| const dynamicImport = new Function('module', 'return import(module)'); | ||
| ({ fromMarkdown } = await dynamicImport('mdast-util-from-markdown')); | ||
| ({ mdxjs } = await dynamicImport('micromark-extension-mdxjs')); | ||
| ({ mdxFromMarkdown } = await dynamicImport('mdast-util-mdx')); | ||
| unistUtilVisit = await dynamicImport('unist-util-visit'); | ||
| }); |
There was a problem hiding this comment.
question: could we change this file's extension to .mts so that we can use ESM and avoid this hack?
There was a problem hiding this comment.
Ah, I've tried it!
But if we change it to .mts, we get a bunch of other issues:
- we need to add
.jsextension to relative imports because ofmoduleResolution - we have to remove these
.jsextensions in jest viamoduleNameMapper(jest does not load these files withoutmoduleNameMapper) - these imported files contain all exported members in the
defaultproperty, so I addedinteropDefault
Here I tried to address all of them - auvred@020b12c (yarn lint, yarn test and yarn typecheck pass locally)
I'm not sure which solution is better (I don't like both, though 😄)
There was a problem hiding this comment.
Ugh jest is becoming a burden.
Might be about time to consider migrating to something else that supports ESM better.
PR Checklist
Overview