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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correct file ignore check for node_modules #532

Closed

Conversation

andyrooger
Copy link

@andyrooger andyrooger commented Apr 17, 2023

Fixes issue where all node_modules and non-ts(x) files are linted even if they're explictly ignored by eslint config.
Also correcting file path check on Windows.


#528 fixed most of our problems running on Windows. Unfortunately, it didn't seem to quite fix the code that ignores linting for node_modules.

After some investigation it turns out on Windows, the checkFileToBeIgnored method sometimes receives files as standard Windows paths (\) but sometimes with forward slashes.

This should fix the issue properly. Apologies to @artola for contributing the dodgy fix to your PR 馃檪

@andyrooger andyrooger marked this pull request as draft April 28, 2023 12:35
@andyrooger
Copy link
Author

andyrooger commented Apr 28, 2023

Edit: Assuming the intention is to hardcode ignoring node_modules and non-ts for performance

OK, we've been running with this on a few machines for a while. It turns out this does fix the check, but the behaviour doesn't seem right. It looked like it worked because it takes maybe an hour before the lint errors start appearing in the IDE (it has a lot to churn through)

If checkFileToBeIgnored determines a file to be from node_modules or not a TS file, then it skips the check and always lints it, even it's it's explicitly listed as ignored in the eslint config.

@Quramy Do you remember what the initial intention of this code was?
@artola Or do you have any insights into it?

I'm running this in VS Enterprise, does that make a difference to which files pass through here? e.g. Does VS code not try to lint node_modules so we didn't need to check?

@andyrooger andyrooger changed the title fix: Correct file ignore check for node_modules on Windows fix: Correct file ignore check for node_modules Apr 28, 2023
@andyrooger andyrooger marked this pull request as ready for review April 28, 2023 21:49
@andyrooger andyrooger marked this pull request as draft April 29, 2023 00:18
@andyrooger
Copy link
Author

Closing in favour of #543

@andyrooger andyrooger closed this May 1, 2023
@andyrooger andyrooger deleted the node-modules-check-windows branch May 1, 2023 23:39
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

Successfully merging this pull request may close these issues.

None yet

1 participant