Skip to content
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

await-interactions: false positive and bad fix if await result is passed to expect in one line #136

Open
jrencz opened this issue Aug 2, 2023 · 1 comment

Comments

@jrencz
Copy link

jrencz commented Aug 2, 2023

Describe the bug
Rule reports problem on following code:

const promise = new Promise<{foo: string}>((resolve) => {
  setTimeout(() => {
    resolve({foo: 'bar'});
  }, 500);
});

expect((await promise).foo).toBe('bar');

if it's written this way:

const promise = new Promise<{foo: string}>((resolve) => {
  setTimeout(() => {
    resolve({foo: 'bar'});
  }, 500);
});

const {foo} = await promise;
expect(foo).toBe('bar');

rule doesn't complain.

Fix is proposed to prepend expect((await promise).foo).toBe('bar') which is incorrect as it doesn't need that in the first place.

That's true Jest expect can be awaited, but then

expect(promise).toBe(resolveValue)

should become

await expect(promise).resolves.toBe(resolveValue)

and fixer just prepends the await keyword

Additional context

@jrencz
Copy link
Author

jrencz commented Aug 2, 2023

After a brief look into the rule code I can narrow the cause of what I see as a problem down to this section:

if (
isMemberExpression(expr.callee) &&
isCallExpression(expr.callee.object) &&
isIdentifier(expr.callee.object.callee) &&
isIdentifier(expr.callee.property) &&
expr.callee.object.callee.name === 'expect'
) {
return expr.callee.property
}

There's a design decision to enforce all expect calls to be awaited, but it's not even covered fully (e.g. it won't trigger the rule if it's expect(…).not.toBe(…) where the pattern of CallExpression>MemberExpression>CallExpression, which this chunk above detects is not preserved)

I went through the history of changes, of the rule, and I see that expect initially was one of FUNCTIONS_TO_BE_AWAITED but then some refinements came, but I'm not sure why was it considered as "it needs to be awaited" at all - it's not at all obvious it should

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant