Dev - Lint is running on node_modules

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the Project on VsCode
2. Open a terminal
3. Run: npm run lint
4. Notice multiple errors coming from node modules files

Expected Result:

Lint should not run on node_modules

Actual Result:

Lint is running on node_modules

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: Latest Main
Reproducible in staging?: N
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @rayane-djouah
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690636757149339

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @flaviadefaria (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[mvtglobally]

Proposal from Contributor @rayane-djouah

Proposal

Please re-state the problem that we are trying to solve in this issue.

In our current configuration of the .eslintrc.js file, we are only instructing ESLint to ignore the node_modules/.bin/ and node_modules/.cache/ directories. However, we are not ignoring the entirety of the node_modules directory. As a result, this slows down lint execution significantly and also causes ESLint to display error messages originating from the node_modules directory.
This can be verified here:

App/.eslintrc.js

Line 5 in 30fc9bf

ignorePatterns: ['!.*', 'src/vendor', '.github/actions/**/index.js', 'desktop/dist/*.js', 'dist/*.js', 'node_modules/.bin/**', 'node_modules/.cache/**', '.git/**'],

What is the root cause of that problem?

The root cause of this problem is the missing 'node_modules/**' pattern in the ignorePatterns array of the .eslintrc.js configuration file. Without this pattern, ESLint does not know to ignore all files and directories inside node_modules, leading to slower execution and unnecessary error messages.

What changes do you think we should make in order to solve the problem?

To solve this problem, we should add 'node_modules/' to the ignorePatterns array in the .eslintrc.js file. This pattern tells ESLint to ignore all files and directories recursively inside the node_modules directory. By implementing this change, we can speed up ESLint execution and avoid error messages from the node_modules directory.
ignorePatterns: ['!.*', 'src/vendor', '.github/actions//index.js', 'desktop/dist/.js', 'dist/.js', 'node_modules/', '.git/'],

What alternative solutions did you explore? (Optional)

[honnamkuan]

I believe this issue is resolved after #20179 PR merge (adding .eslintignore)

[flaviadefaria]

@hayata-suenaga since you worked on the issue with the linked PR do you agree with the above?

[hayata-suenaga]

wow we actually wasn't ignoring node_module at all for linter all this time???

@flaviadefaria anyway, yes that PR should fix the issue

[flaviadefaria]

Cool so closing this!