feat: remove RuleTester in /utils in favour of the new /rule-tester package
#6816
Conversation
|
Thanks for the PR, @bradzacher! 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. |
|
For future reviewing reference... [...document.querySelectorAll(".file-header:has(.rgh-seen--11765709499)")]
.filter(e => e.textContent.includes("packages/eslint-plugin/tests/rules/") && e.textContent.includes(".test.ts"))
.forEach(e => e.querySelector("form label").click()) |
| } from '@typescript-eslint/utils/eslint-utils/rule-tester'; | ||
|
|
||
| export { batchedSingleLineTests, getFixturesRootDir }; | ||
| // TODO - migrate the codebase off of this utility |
There was a problem hiding this comment.
[Docs] Can you include a tracking GH issue?
| @@ -63,6 +63,9 @@ function isDescribeWithSkip( | |||
| ); | |||
| } | |||
|
|
|||
| /** | |||
| * @deprecated use `@typescript-eslint/rule-tester` instead | |||
There was a problem hiding this comment.
[Docs] Can you also file a delection tracking issue & link to it here? I'm guessing it'll be a v7 breaking change.
c301a8e to
3a2d8da
Compare
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
RuleTester in /utils in favour of the new rule-tester package
RuleTester in /utils in favour of the new rule-tester packageRuleTester in /utils in favour of the new rule-tester package
|
re-requesting reviews because I've made this a breaking change. |
RuleTester in /utils in favour of the new rule-tester packageRuleTester in /utils in favour of the new /rule-tester package
| errors: [ | ||
| { | ||
| messageId: 'unsafeArraySpread', | ||
| line: 2, | ||
| column: 12, | ||
| endColumn: 25, | ||
| }, | ||
| { | ||
| messageId: 'unsafeArraySpread', | ||
| line: 3, | ||
| column: 12, | ||
| endColumn: 28, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
is there a reason why we are not preserving loine/col/endCol ?
its a good practice to add those to validate if error message is reported in correct place
There was a problem hiding this comment.
because I couldn't be bothered remapping each one before and after the prettier format.
it already took long enough to extract and convert things away from the utility as it is.
soon™️ I'll implement snapshot testing in the fork which will snapshot an error underline and will make the line/col assertions unnecessary cruft anyways.
packages/website/blog/2023-03-13-announcing-typescript-eslint-v6-beta.md
Outdated
Show resolved
Hide resolved
…v6-beta.md Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
BREAKING CHANGE:
Removes
RuleTesterfrom the@typescript-eslint/utils/eslint-utilspackage in favour of the new@typescript-eslint/rule-testerpackage.PR Checklist
Overview
Migrates our codebase to use the package added in #6777 as a thorough test of the fork to make sure it works as expected.
filenameconfig. The old rule tester didfile.tsxfor JSX files - which actually didn't work at all because TS wouldn't pick up bothfile.tsandfile.tsx.batchedSingleLineTestsutil from some places - but it's probably worth removing it entirely in future (once I add snapshot tests we shouldn't need it)