svelte-check fails if importing TS in monorepo

[jakzo]

Describe the bug

In a monorepo setup importing a Typescript file from outside a package's tsconfig.json rootDir file Typescript will complain with the following when running svelte-check:

File '/svelte-ts-monorepo-issue/packages/dep/index.ts' is not under 'rootDir' '/svelte-ts-monorepo-issue/packages/app'. 'rootDir' is expected to contain all source files.
  The file is in the program because:
    Imported via "dep" from file '/svelte-ts-monorepo-issue/packages/app/index.ts' with packageId 'dep/[email protected]'
    Root file specified for compilation

This happens even doing an absolute import of a monorepo dependency (eg. import "dep") which is symlinked into node_modules. Running tsc does not complain in this case, only when running svelte-check does it complain.

Reproduction

Clone and run commands from readme: https://github.com/jakzo/svelte-ts-monorepo-issue

Expected behaviour

svelte-check shows the same Typescript errors as tsc and allows importing from symlinked dependencies in node_modules.

System Info

• OS: MacOS
• IDE: VSCode

Which package is the issue about?

svelte-check

Additional Information, eg. Screenshots

No response

[jakzo]

I tried debugging this just then and I think the issue stems from here: https://github.com/sveltejs/language-tools/blob/master/packages/language-server/src/svelte-check.ts#L229

The Typescript compiler program when initially parsing the program processes the import "dep" statement and finds dep/index.ts by searching in node_modules and marks it as such which is why tsc doesn't complain because isSourceFileFromExternalLibrary() returns true for dep/index.ts, meaning the file will not be emitted and does not need to be under rootDir.

However when svelte-check gets the list of files in the program Typescript returns the non-symlinked path to dep/index.ts which is not in node_modules and then when svelte-check gets its diagnostics using the non-node_modules path Typescript sees it as not an external library and complains about rootDir.

[dummdidumm]

The problem is that our getScriptFileNames includes all files, not just the root file names, which apparently it only should contain. I'm not sure if that finding is correct (I dug around the TS code base) and how we could determine that something is indeed a root file. @jasonlyu123 can you confirm that it should only contain root files, and/or do you have further info/details/ideas?

[jasonlyu123]

Yeah. They should only contain root files. That's the project files in our code base plus client-opened files. If the opened files are not there, TypeScript throws an error for missing source files when requesting language features for those files.

[dummdidumm]

What I dug up is that the language service's getScriptFileNames seems to only be called right here: https://github.com/microsoft/TypeScript/blob/7c417bfd1d14326575a2f08f8b6013a8c10d9f9e/src/services/services.ts#L1640

And when it does that in the context of svelte-check - where we return all the other files from node_modules as well because of our flawed getScriptFileNames implementation - then there's some logic running along the lines of "oh so there are new root files, we need to adjust some map called sourceFilesFoundSearchingNodeModules accordingly", and when we later want to filter out checking diagnostics for files found in node modules there are then false negatives because dep/index.ts is now part of the root files and therefore isSourceFileFromExternalLibrary() returns false when it should return true.

So it sounds like we should filter the list of script file names before returning it - something like "is this not inside node_modules or opened in the client". Does that make sense, or will this result in breakage for the IDE case?

[jasonlyu123]

I think we can only return project files and client files. Maybe instead of tracking client files in the document manager, we track it in a property of Document so it can be carried over to the script snapshot. But that causes some tests to fail because we're relying on getSnapshot to create a new root file and invalidate failed module resolution. So we probably need to either create a getClientSnpshot or documentManager.openClientFiles to mark files as client files outside the LSP open document request.

[dummdidumm]

Could you open a draft PR with what you tried which isn't working, just so I can take a closer look at what exactly you tried?