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

Bug: no-sync fail positive #354

Closed
1 task done
RebeccaStevens opened this issue Oct 10, 2024 · 3 comments · Fixed by #386
Closed
1 task done

Bug: no-sync fail positive #354

RebeccaStevens opened this issue Oct 10, 2024 · 3 comments · Fixed by #386

Comments

@RebeccaStevens
Copy link

Environment

Node version: 22.9.0
npm version: 10.8.3
ESLint version: 9.12.0
eslint-plugin-n version: 17.11.0
Operating System: Nobara 40

What rule do you want to report?

no-sync

Link to Minimal Reproducible Example

https://eslint-online-playground.netlify.app/#eNp9UE1PwzAM/SuWT5u0dRLiVMQNfgFHzCFK3aqQOlE+pk1V/ztJ08G4cHFi+/m9Z88YvD7xRU3OcPMZsMVxctZHmOG171lHWKD3dgJCXnPCJxISbSVEOCuTGJ43aOOTvF1F70jgVgpJa+Zu9/iwJ9nnUTwgBzNKbDJFPw5/ROVHa4UcnUnDKEfZRPmyojruVTIR3ovOXAJARYY2+xZYDrXok+FSqhlkXjmJPYbskbAtMt5bT1j7S3ly+KgundJfaihHsZIdriSE8eq4zk62y/SEqxZhx+cXdiwdix45FMym+3u5MmZU5JD/m8Xbqv827+5wj9r8Lrh8Ax6jlbM=

What did you expect to happen?

No error, as Effect.runSync is not part of the node api.

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

I've got a package that you can use to detect where types are coming from: ts-declaration-location

With this, you can make it so only types from @types/node are marked as violations.
Note: this package is also being used by eslint-plugin-functional.

@scagood
Copy link

scagood commented Oct 10, 2024

mmm, the rule was not meant to detect only node APIs, it was meant to detect anything that could be synchronous 🤔

The only test for this rule is effectively this selector:

const allowedAtRootLevelSelector = [
// fs.readFileSync()
":function MemberExpression > Identifier[name=/Sync$/]",
// readFileSync.call(null, 'path')
":function MemberExpression > Identifier[name=/Sync$/]",
// readFileSync()
":function :not(MemberExpression) > Identifier[name=/Sync$/]",
].join(",")

@scagood
Copy link

scagood commented Oct 10, 2024

🤔 I wonder if we should add an ignores to this rule too.

I noticed a similar thing here: #341 (comment)

@aladdin-add
Copy link

👍 to adding a new option ignores.

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