False positive for await-async-utils
iantocristian opened this issue ยท 7 comments
Have you read the Troubleshooting section?
Yes
Plugin version
v5.10.1
ESLint version
v8.33.0
Node.js version
v18.12.1
package manager and version
npm 9.4.2
Operating system
macOS Ventura 13.2
Bug description
This appears to be a regression introduced with the latest patch release, v5.10.1. Not present in v5.10.0.
False positive rule await-async-utils triggered by inline async function definition on other variables and/or definitions.
Promise returned from x wrapper over async util must be handled but x is not a function.
Steps to reproduce
This code triggers a false positive on expectedValue:
import React from 'react';
import { render, act } from '@testing-library/react';
function wait(ms) {
return new Promise((resolve) => {
setTimeout(resolve, ms);
});
}
const waitWithAct = async (timeout) => {
await act(async () => await wait(timeout));
};
describe('Component', () => {
const mock = jest.fn();
it('test', async () => {
let Component = () => {
mock(1);
return <div />;
};
render(<Component />);
await waitWithAct(500);
const expectedValue = 1;
expect(mock).toHaveBeenCalledWith(expectedValue);
});
});Rewriting the inline async function fixes the false positive:
import React from 'react';
import { render, act } from '@testing-library/react';
function wait(ms) {
return new Promise((resolve) => {
setTimeout(resolve, ms);
});
}
const waitWithAct = async (timeout) => {
await act(async () => {
await wait(timeout);
});
};
describe('Component', () => {
const mock = jest.fn();
it('test', async () => {
let Component = () => {
mock(1);
return <div />;
};
render(<Component />);
await waitWithAct(500);
const expectedValue = 1;
expect(mock).toHaveBeenCalledWith(expectedValue);
});
});Another case of false positive, this results in false positives reported for mock and jest.fn:
import React from 'react';
import { render, act } from '@testing-library/react';
function wait(ms) {
return new Promise((resolve) => {
setTimeout(resolve, ms);
});
}
const waitWithAct = async (timeout) => {
const fn = async () => await wait(timeout);
await act(fn);
};
describe('Component', () => {
const mock = jest.fn();
it('test', async () => {
let Component = () => {
mock(1);
return <div />;
};
render(<Component />);
await waitWithAct(500);
const expectedValue = 1;
expect(mock).toHaveBeenCalledWith(expectedValue);
});
});Error output/screenshots
No response
ESLint configuration
module.exports = {
env: {
es6: true,
},
parserOptions: {
ecmaVersion: 2021,
ecmaFeatures: {
jsx: true,
},
sourceType: 'module',
},
globals: {
process: true,
module: true,
},
extends: ['plugin:prettier/recommended'],
plugins: ['prettier', 'eslint-plugin-no-only-tests', 'deprecate', 'jest', 'testing-library', 'cypress'],
overrides: [
{
files: ['*.test.jsx', '*.test.js', 'jest/setup-tests.js'],
extends: ['plugin:jest/recommended', 'plugin:testing-library/react'],
},
],
};Rule(s) affected
testing-library/await-async-utils
Do you want to submit a pull request to fix this bug?
No
Thanks for reporting @iantocristian! This is a regression introduced in the last version indeed.
Pinging @patriscus in case you can check this issue.
I'm sorry to see that I've caused some issues ๐ I had a quick look:
The first example seems to be related to the isAssigningKnownAsyncFunctionWrapper I introduced in my latest commit. Using getDeepestIdentifierNode(node.init)?.name ?? '' actually returns true for expectedValue, since we fallback to the default (''). Interestingly, '' is actually a part of the functionWrapperNames since the getFunctionName(innerFunction) call within the detectAsyncUtilWrapper function pushes an empty string into the array.
I very quickly added a safeguard to not push empty strings into the array, and the tests seem to pass (need to verify it a bit more thoroughly).
The second example is more complex in my eyes. This code fails even before my commit - my theory is that since we push wrapper names, we don't really check scopes. So in the example provided:
const waitWithAct = async (timeout) => {
const fn = async () => await wait(timeout);
await act(fn);
};We will push fn into the functionWrapperNames. Later, by using jest.fn(), it's mistakenly flagged as a wrapper around a util because fn matches. So I guess we need to somehow get the "true" variables/references, instead of "just" comparing names.
I could be wrong, though.
I will try to provide a PR for the first issue by tomorrow's EOD.
About the second example, I don't know if it makes sense to do it in this issue. It seems to me that this is a separate topic; hence I wanted to ask if it should be extracted into a separate issue. I will for sure not be able to solve it within the next 2 days, and I'm not available for a couple of days (vacation). I can, however, pick it up after I'm back (would just give a quick ping to let you know).
Please let me know what you think @Belco90.
Thanks for addressing this issue!
@patriscus Better to address the issues in separate PRs!
๐ This issue has been resolved in version 5.10.2 ๐
The release is available on:
Your semantic-release bot ๐ฆ๐
๐ This issue has been resolved in version 6.0.0-alpha.15 ๐
The release is available on:
Your semantic-release bot ๐ฆ๐