Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat: removeRuleTester in/utils in favour of the new/rule-tester package#6816
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 onhttps://opencollective.com/typescript-eslint. |
nx-cloudbot commentedApr 2, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
JoshuaKGoldberg commentedApr 3, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🎊
| } 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[Docs] Can you include a tracking GH issue?
| @@ -63,6 +63,9 @@ function isDescribeWithSkip( | |||
| ); | |||
| } | |||
| /** | |||
| * @deprecated use `@typescript-eslint/rule-tester` instead | |||
JoshuaKGoldbergApr 3, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[Docs] Can you also file a delection tracking issue & link to it here? I'm guessing it'll be a v7 breaking change.
c301a8e to3a2d8daComparenetlifybot commentedApr 27, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site settings. |
RuleTester in/utils in favour of the new rule-tester packageRuleTester in/utils in favour of the new rule-tester packageRuleTester in/utils in favour of the newrule-tester packagere-requesting reviews because I've made this a breaking change. |
RuleTester in/utils in favour of the newrule-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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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
bradzacherApr 27, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
packages/website/blog/2023-03-13-announcing-typescript-eslint-v6-beta.md OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
…v6-beta.mdCo-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
Uh oh!
There was an error while loading.Please reload this page.
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)