Skip to content

Conversation

@octref
Copy link
Member

@octref octref commented Apr 26, 2017

/cc @sandersn

For #131 and alike, this seems to happen when a project is large.

In one of my project I was able to repro it, and while debugging I found:

https://github.com/octref/vetur/blob/master/server/src/modes/javascriptMode.ts#L50-L67

  const { createLanguageServiceSourceFile, updateLanguageServiceSourceFile } = createUpdater();
  (ts as any).createLanguageServiceSourceFile = createLanguageServiceSourceFile;
  (ts as any).updateLanguageServiceSourceFile = updateLanguageServiceSourceFile;
  const configFilename = ts.findConfigFile(workspacePath, ts.sys.fileExists, 'tsconfig.json') ||
    ts.findConfigFile(workspacePath, ts.sys.fileExists, 'jsconfig.json');
  const configJson = configFilename && ts.readConfigFile(configFilename, ts.sys.readFile).config || {};
  const parsedConfig = ts.parseJsonConfigFileContent(configJson,
    ts.sys,
    workspacePath,
    compilerOptions,
    configFilename,
    undefined,
    [{ extension: 'vue', isMixedContent: true }]);

  // This files is an array of over 6k items with all js/ts/vue files in current workspace, including those in node_modules
  const files = parsedConfig.fileNames; 

  compilerOptions = parsedConfig.options;
  compilerOptions.allowNonTsExtensions = true;

Then later in here:

https://github.com/octref/vetur/blob/master/server/src/modes/javascriptMode.ts#L89-L109

    resolveModuleNames(moduleNames: string[], containingFile: string): ts.ResolvedModule[] {
      // This is run 6k times, one time for each file
      return moduleNames.map(name => {
        if (path.isAbsolute(name) || !isVue(name)) {
          return ts.resolveModuleName(name, containingFile, compilerOptions, ts.sys).resolvedModule;
        }
        else {
          const uri = Uri.file(path.join(path.dirname(containingFile), name));
          const resolvedFileName = uri.fsPath;
          if (ts.sys.fileExists(resolvedFileName)) {
            const doc = docs.get(resolvedFileName) ||
              jsDocuments.get(TextDocument.create(uri.toString(), 'vue', 0, ts.sys.readFile(resolvedFileName)));
            return {
              resolvedFileName,
              extension: doc.languageId === 'typescript' ? ts.Extension.Ts : ts.Extension.Js,
            };
          }
        }
      });
    },

This function is called for each files. So for the whole worksapce it's called 6k times, one time for each js/ts/vue file, and then for each file TS was resolving all modules in them...

I don't know if this PR is the right approach, but it seems to solve the problem.

@sandersn
Meanwhile in #136 (comment) you said

I think there may be some tsconfig settings to limit how deeply the compiler follows module resolution inside node_modules.

Do you mean maxNodeModuleJsDepth? That might help solving this issue, but how is the depth calculated?

@sandersn
Copy link
Contributor

maxNodeModuleJsDepth means the number of times to follow import within imported .js files that are under node_modules. So if your code imports 'libraryA', libraryA is at depth 0. When 'libraryA' imports 'libraryB', libraryB is at depth 1, and so on if libraryB imports more modules. Module resolution stops after it hits the max and adds further imports to a list of "elided imports", which basically means that the module is treated as type any.

I confirmed with @mhegazy that if not provided it defaults to 0, so it should not make much difference.

This is different from excluding node_modules. That stops the compiler from its normal, non-import search for source files, which sounds like exactly what we want here. maxNodeModuleJsDepth only applies when somebody has explicitly said import X from 'library' and the compiler finds library inside node_modules.

@octref octref merged commit 0dbb799 into master Apr 27, 2017
@octref
Copy link
Member Author

octref commented Apr 27, 2017

@sandersn Thanks for the info!

@octref
Copy link
Member Author

octref commented Apr 27, 2017

@sandersn

Is there anyway to exclude node_modules even in subdirectories of workspaces?

Tried:

exclude: ['node_modules', '**/node_modules']

No effect, still breaks on https://github.com/mrgodhani/rss-reader per #131 (comment). files still include everything under app/node_modules/.


OK, just figured out this works:

exclude: ['node_modules', '**/node_modules/*']

This seems to match everything under app/node_modules. Still, a bit surprising that **/node_modules wouldn't match everything under app/node_modules -- would you say that's a bug for TS?

@octref octref deleted the cpu-memory-spike branch April 27, 2017 04:15
@sandersn
Copy link
Contributor

I don't actually know the glob code. @rbuckton, can you comment whether the above **/node_modules/* is according to spec?

Here are the PRs in case you want to look through the code yourself:
microsoft/TypeScript#8841
microsoft/TypeScript#5980

octref added a commit that referenced this pull request Nov 22, 2017
Exclude node_modules in module resolution
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.

3 participants