-
Notifications
You must be signed in to change notification settings - Fork 150
feat(prefer-wait-for): new rule prefer-wait-for #88
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
d8a6818
3dc56de
ea8fcce
fe39525
550155f
d550a23
4e21af6
0c4ae1e
506f956
8e2ba9a
e69fb42
938e907
c5b413a
df677fe
0c698cd
a841ffe
c2be2ec
b410020
41414a4
99dac28
12980e8
623f88b
82fa2df
03b352d
67c1f5b
158be04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# Use `waitFor` instead of deprecated wait methods (prefer-wait-for) | ||
|
||
Since dom-testing-library v7 new `waitFor` async util has been added. This new method satisfies the use cases of `wait`, `waitForElement`, and `waitForDomChange`, so those have been deprecated. | ||
|
||
## Rule Details | ||
|
||
This rule aims to use `waitFor` async util rather than previous deprecated ones. | ||
|
||
Deprecated wait async utils are: | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- `wait` | ||
- `waitForElement` | ||
- `waitForDomChange` | ||
|
||
> This rule will auto fix deprecated async utils for you, including necessary empty callback for `waitFor`. This means `wait();` will be replaced with `waitFor(() => {});` | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Examples of **incorrect** code for this rule: | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```js | ||
const foo = async () => { | ||
// new waitFor method | ||
await waitFor(() => {}); | ||
|
||
// previous waitForElementToBeRemoved is not deprecated | ||
await waitForElementToBeRemoved(() => {}); | ||
}; | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```js | ||
const foo = async () => { | ||
await wait(); | ||
await wait(() => {}); | ||
await waitForElement(() => {}); | ||
await waitForDomChange(); | ||
await waitForDomChange(mutationObserverOptions); | ||
await waitForDomChange({ options: true }); | ||
}; | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
When using dom-testing-library (or any other Testing Library relying on dom-testing-library) prior to v7. | ||
|
||
## Further Reading | ||
|
||
- [dom-testing-library v7 release](https://github.com/testing-library/dom-testing-library/releases/tag/v7.0.0) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
'use strict'; | ||
|
||
const { getDocsUrl } = require('../utils'); | ||
|
||
module.exports = { | ||
meta: { | ||
type: 'suggestion', | ||
docs: { | ||
description: 'Use `waitFor` instead of deprecated wait methods', | ||
category: 'Best Practices', | ||
recommended: false, | ||
url: getDocsUrl('prefer-wait-for'), | ||
}, | ||
messages: { | ||
preferWaitFor: '`{{ methodName }}` is deprecated in favour of `waitFor`', | ||
}, | ||
fixable: 'code', | ||
schema: [], | ||
}, | ||
|
||
create: function(context) { | ||
const reportWait = node => { | ||
context.report({ | ||
node: node, | ||
messageId: 'preferWaitFor', | ||
data: { | ||
methodName: node.name, | ||
}, | ||
fix: fixer => { | ||
const { parent } = node; | ||
const [arg] = parent.arguments; | ||
|
||
if (arg) { | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return fixer.replaceText(node, 'waitFor'); | ||
} | ||
return fixer.replaceText(parent, 'waitFor(() => {})'); | ||
}, | ||
}); | ||
}; | ||
|
||
const reportWaitForDomChange = node => { | ||
context.report({ | ||
node: node, | ||
messageId: 'preferWaitFor', | ||
data: { | ||
methodName: node.name, | ||
}, | ||
fix: fixer => { | ||
const { parent } = node; | ||
const [arg] = parent.arguments; | ||
if (arg) { | ||
return [ | ||
fixer.replaceText(node, 'waitFor'), | ||
fixer.insertTextBefore(arg, `() => {}, `), | ||
]; | ||
} | ||
return fixer.replaceText(parent, 'waitFor(() => {})'); | ||
}, | ||
}); | ||
}; | ||
|
||
return { | ||
'CallExpression > Identifier[name=wait]'(node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the user is using a custom There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. I think it's too much from where it was imported? But I don't know what to do to be honest. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Obviously it would be better to check where it's imported from, but we have been checking this in other rules without caring about it. Could be an improvement for a separated ticket for all the rules within the plugin? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the other hand... It would be interesting to check from where it's imported so we can also replace the import itself. Otherwise, the fix of this rule could leave a broken file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest keep it like this for now and wait for users to ask for it, in case it's a valid use case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless it's not too complicated to implement of course 😅 |
||
reportWait(node); | ||
}, | ||
'CallExpression > Identifier[name=waitForElement]'(node) { | ||
reportWait(node); | ||
}, | ||
'CallExpression > Identifier[name=waitForDomChange]'(node) { | ||
reportWaitForDomChange(node); | ||
}, | ||
}; | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
'use strict'; | ||
|
||
const rule = require('../../../lib/rules/prefer-wait-for'); | ||
const RuleTester = require('eslint').RuleTester; | ||
|
||
const ruleTester = new RuleTester({ | ||
parserOptions: { ecmaVersion: 2018, sourceType: 'module' }, | ||
}); | ||
ruleTester.run('prefer-wait-for', rule, { | ||
valid: [ | ||
{ | ||
code: `async () => { | ||
await waitFor(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await waitForElementToBeRemoved(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await waitForSomethingElse(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `import { wait } from 'some-testing-library'`, | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
{ | ||
code: `import { waitForElement } from 'some-testing-library'`, | ||
}, | ||
{ | ||
code: `import { waitForDomChange } from 'some-testing-library'`, | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
], | ||
|
||
invalid: [ | ||
{ | ||
code: `async () => { | ||
await wait(); | ||
}`, | ||
errors: [ | ||
{ | ||
messageId: 'preferWaitFor', | ||
line: 2, | ||
column: 15, | ||
}, | ||
], | ||
output: `async () => { | ||
await waitFor(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await wait(() => {}); | ||
}`, | ||
errors: [ | ||
{ | ||
messageId: 'preferWaitFor', | ||
line: 2, | ||
column: 15, | ||
}, | ||
], | ||
output: `async () => { | ||
await waitFor(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await waitForElement(() => {}); | ||
}`, | ||
errors: [ | ||
{ | ||
messageId: 'preferWaitFor', | ||
line: 2, | ||
column: 15, | ||
}, | ||
], | ||
output: `async () => { | ||
await waitFor(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await waitForDomChange(); | ||
}`, | ||
errors: [ | ||
{ | ||
messageId: 'preferWaitFor', | ||
line: 2, | ||
column: 15, | ||
}, | ||
], | ||
output: `async () => { | ||
await waitFor(() => {}); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await waitForDomChange(mutationObserverOptions); | ||
}`, | ||
errors: [ | ||
{ | ||
messageId: 'preferWaitFor', | ||
line: 2, | ||
column: 15, | ||
}, | ||
], | ||
output: `async () => { | ||
await waitFor(() => {}, mutationObserverOptions); | ||
}`, | ||
}, | ||
{ | ||
code: `async () => { | ||
await waitForDomChange({ options: true }); | ||
}`, | ||
errors: [ | ||
{ | ||
messageId: 'preferWaitFor', | ||
line: 2, | ||
column: 15, | ||
}, | ||
], | ||
output: `async () => { | ||
await waitFor(() => {}, { options: true }); | ||
}`, | ||
}, | ||
], | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.