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

Ignore all package paths #2445

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DavidArchibald
Copy link
Contributor

This is based off of a real issue I faced; I was getting lints for files in node_modules, specifically in .ts files that I can't change anyways. It also doesn't show up when I run tsc on my own .ts files even if they import the same erroring .ts files and even if I use svelte2tsx to use the same built ts file.

I hope there isn't a reason I'm missing for keeping .ts files. In this code path all node_modules diagnostics are removed already:

if (
(document.getFilePath()?.includes('/node_modules/') ||
document.getFilePath()?.includes('\\node_modules\\')) &&
// Sapper convention: Put stuff inside node_modules below src
!(
document.getFilePath()?.includes('/src/node_modules/') ||
document.getFilePath()?.includes('\\src\\node_modules\\')
)
) {
// Don't return diagnostics for files inside node_modules. These are considered read-only (cannot be changed)
// and in case of svelte-check they would pollute/skew the output
return [];
}

And in tsc itself it ignores items in the commonPackageFolders:
https://github.com/microsoft/TypeScript/blob/ec7ff812c1b8e4c8f34901da900f9c933b6dfe2a/src/compiler/utilities.ts#L9353-L9355

So I hope this helps rather than being the wrong fix.

@jasonlyu123
Copy link
Member

I don't think the correct fix. tsc does check ts files in the node_modules so the inconsistency might come from another place. Please create an issue first with a reproduction so we can check where the problem is.

@DavidArchibald
Copy link
Contributor Author

I figured out that it's because it's inside an internal folder in node_modules.

See #2446

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.

2 participants