Override isExternalLibraryImport as needed; re-add realpath (#970)

* Re-add "realpath" to LanguageServiceHost
Implement resolveModuleNames that forces modules to be considered internal (not from an external library) when we need them to be emitted

* fix linter failures

* fix failure on old ts version

* Add $$ts-node-root.ts synthetic root file, which `/// <reference`'s all require()d files to
force them to be included in compilation without modifying the rootFiles array

* Preserve path casing in /// <references

* fix

* Revert $$ts-node-root changes, limiting only to the isExternal* changes

* Add new resolver behavior to the CompilerHost codepath

* WIP

* add tests; plus fixes

* Fix tests

* Code-reviewing myself

* fix tests

* add missing test files

* fix tests

* fix tests

* fix linter

* fix tests on windows

* remove comma from the diff

* Update package.json

* adding handling of @Scoped modules in bucketing logic; adds tests

Author: cspotcode

development-docs/isExternalLibraryImport.md

@@ -0,0 +1,63 @@
+## How we override isExternalLibraryImport
+
+`isExternalLibraryImport` is a boolean returned by node's module resolver that is `true`
+if the target module is inside a `node_modules` directory.
+
+This has 2x effects inside the compiler:
+a) compiler refuses to emit JS for external modules
+b) increments node_module depth +1, which affects `maxNodeModulesJsDepth`
+
+If someone `require()`s a file inside `node_modules`, we need to override this flag to overcome (a).
+
+### ts-node's behavior
+
+- If TS's normal resolution deems a file is external, we might override this flag.
+  - Is file's containing module directory marked as "must be internal"?
+    - if yes, override as "internal"
+    - if no, track this flag, and leave it as "external"
+
+When you try to `require()` a file that's previously been deemed "external", we mark the entire module's
+directory as "must be internal" and add the file to `rootFiles` to trigger a re-resolve.
+
+When you try to `require()` a file that's totally unknown to the compiler, we have to add it to `rootFiles`
+to trigger a recompile.  This is a separate issue.
+
+### Implementation notes
+
+In `updateMemoryCache`:
+- If file is not in rootFiles and is not known internal (either was never resolved or was resolved external)
+  - mark module directory as "must be internal"
+  - add file to rootFiles to either pull file into compilation or trigger re-resolve (will do both)
+
+TODO: WHAT IF WE MUST MARK FILEA INTERNAL; WILL FILEB AUTOMATICALLY GET THE SAME TREATMENT?
+
+TODO if `noResolve`, force adding to `rootFileNames`?
+
+TODO if `noResolve` are the resolvers called anyway?
+
+TODO eagerly classify .ts as internal, only use the "bucket" behavior for .js?
+- b/c externalModule and maxNodeModulesJsDepth only seems to affect typechecking of .js, not .ts
+
+### Tests
+
+require() .ts file where TS didn't know about it before
+require() .js file where TS didn't know about it before, w/allowJs
+import {} ./node_modules/*/.ts
+import {} ./node_modules/*/.js w/allowJs (initially external; will be switched to internal)
+import {} ./node_modules/*/.ts from another file within node_modules
+import {} ./node_modules/*/.js from another file within node_modules
+require() from ./node_modules when it is ignored; ensure is not forced internal and maxNodeModulesJsDepth is respected (type info does not change)
+
+### Keywords for searching TypeScript's source code
+
+These may jog my memory the next time I need to read TypeScript's source and remember how this works.
+
+currentNodeModulesDepth
+sourceFilesFoundSearchingNodeModules
+
+isExternalLibraryImport is used to increment currentNodeModulesDepth
+currentNodeModulesDepth is used to put things into sourceFilesFoundSearchingNodeModules
+
+https://github.com/microsoft/TypeScript/blob/ec338146166935069124572135119b57a3d2cd22/src/compiler/program.ts#L2384-L2398
+
+getSourceFilesToEmit / sourceFileMayBeEmitted obeys internal "external" state, is responsible for preventing emit of external modules

scripts/create-merged-schema.ts

@@ -54,7 +54,7 @@ async function main() {
   );
 }
 
-async function getSchemastoreSchema() {
+export async function getSchemastoreSchema() {
   const {data: schemastoreSchema} = await axios.get(
     'https://schemastore.azurewebsites.net/schemas/json/tsconfig.json',
     { responseType: "json" }

src/index.spec.ts

@@ -594,24 +594,52 @@ describe('ts-node', function () {
           return done()
         })
       })
-
-      it('should give ts error for invalid node_modules', function (done) {
-        exec(`${cmd} --compiler-host --skip-ignore tests/from-node-modules/from-node-modules`, function (err, stdout) {
-          if (err === null) return done('Expected an error')
-
-          expect(err.message).to.contain('Unable to compile file from external library')
-
-          return done()
-        })
-      })
     })
 
     it('should transpile files inside a node_modules directory when not ignored', function (done) {
-      exec(`${cmd} --skip-ignore tests/from-node-modules/from-node-modules`, function (err, stdout, stderr) {
+      exec(`${cmdNoProject} --script-mode tests/from-node-modules/from-node-modules`, function (err, stdout, stderr) {
         if (err) return done(`Unexpected error: ${err}\nstdout:\n${stdout}\nstderr:\n${stderr}`)
+        expect(JSON.parse(stdout)).to.deep.equal({
+          external: {
+            tsmri: { name: 'typescript-module-required-internally' },
+            jsmri: { name: 'javascript-module-required-internally' },
+            tsmii: { name: 'typescript-module-imported-internally' },
+            jsmii: { name: 'javascript-module-imported-internally' }
+          },
+          tsmie: { name: 'typescript-module-imported-externally' },
+          jsmie: { name: 'javascript-module-imported-externally' },
+          tsmre: { name: 'typescript-module-required-externally' },
+          jsmre: { name: 'javascript-module-required-externally' }
+        })
         done()
       })
     })
+
+    describe('should respect maxNodeModulesJsDepth', function () {
+      it('for unscoped modules', function (done) {
+        exec(`${cmdNoProject} --script-mode tests/maxnodemodulesjsdepth`, function (err, stdout, stderr) {
+          expect(err).to.not.equal(null)
+          expect(stderr.replace(/\r\n/g, '\n')).to.contain(
+            'TSError: ⨯ Unable to compile TypeScript:\n' +
+            "other.ts(4,7): error TS2322: Type 'string' is not assignable to type 'boolean'.\n" +
+            '\n'
+          )
+          done()
+        })
+      })
+
+      it('for @scoped modules', function (done) {
+        exec(`${cmdNoProject} --script-mode tests/maxnodemodulesjsdepth-scoped`, function (err, stdout, stderr) {
+          expect(err).to.not.equal(null)
+          expect(stderr.replace(/\r\n/g, '\n')).to.contain(
+            'TSError: ⨯ Unable to compile TypeScript:\n' +
+            "other.ts(7,7): error TS2322: Type 'string' is not assignable to type 'boolean'.\n" +
+            '\n'
+          )
+          done()
+        })
+      })
+    })
   })
 
   describe('register', function () {

src/index.ts

@@ -1,10 +1,10 @@
-import { relative, basename, extname, resolve, dirname, join } from 'path'
+import { relative, basename, extname, resolve, dirname, join, isAbsolute } from 'path'
 import sourceMapSupport = require('source-map-support')
 import * as ynModule from 'yn'
 import { BaseError } from 'make-error'
 import * as util from 'util'
 import { fileURLToPath } from 'url'
-import * as _ts from 'typescript'
+import type * as _ts from 'typescript'
 import * as Module from 'module'
 
 /**
@@ -95,6 +95,18 @@ export interface TSCommon {
   formatDiagnosticsWithColorAndContext: typeof _ts.formatDiagnosticsWithColorAndContext
 }
 
+/**
+ * Compiler APIs we use that are marked internal and not included in TypeScript's public API declarations
+ */
+interface TSInternal {
+  // https://github.com/microsoft/TypeScript/blob/4a34294908bed6701dcba2456ca7ac5eafe0ddff/src/compiler/core.ts#L1906-L1909
+  createGetCanonicalFileName (useCaseSensitiveFileNames: boolean): TSInternal.GetCanonicalFileName
+}
+namespace TSInternal {
+  // https://github.com/microsoft/TypeScript/blob/4a34294908bed6701dcba2456ca7ac5eafe0ddff/src/compiler/core.ts#L1906
+  export type GetCanonicalFileName = (fileName: string) => string
+}
+
 /**
  * Export the current version.
  */
@@ -515,6 +527,103 @@ export function create (rawOptions: CreateOptions = {}): Register {
   let getOutput: (code: string, fileName: string) => SourceOutput
   let getTypeInfo: (_code: string, _fileName: string, _position: number) => TypeInfo
 
+  const getCanonicalFileName = (ts as unknown as TSInternal).createGetCanonicalFileName(ts.sys.useCaseSensitiveFileNames)
+
+  // In a factory because these are shared across both CompilerHost and LanguageService codepaths
+  function createResolverFunctions (serviceHost: _ts.ModuleResolutionHost) {
+    const moduleResolutionCache = ts.createModuleResolutionCache(cwd, getCanonicalFileName, config.options)
+    const knownInternalFilenames = new Set<string>()
+    /** "Buckets" (module directories) whose contents should be marked "internal" */
+    const internalBuckets = new Set<string>()
+
+    // Get bucket for a source filename.  Bucket is the containing `./node_modules/*/` directory
+    // For '/project/node_modules/foo/node_modules/bar/lib/index.js' bucket is '/project/node_modules/foo/node_modules/bar/'
+    // For '/project/node_modules/foo/node_modules/@scope/bar/lib/index.js' bucket is '/project/node_modules/foo/node_modules/@scope/bar/'
+    const moduleBucketRe = /.*\/node_modules\/(?:@[^\/]+\/)?[^\/]+\//
+    function getModuleBucket (filename: string) {
+      const find = moduleBucketRe.exec(filename)
+      if (find) return find[0]
+      return ''
+    }
+
+    // Mark that this file and all siblings in its bucket should be "internal"
+    function markBucketOfFilenameInternal (filename: string) {
+      internalBuckets.add(getModuleBucket(filename))
+    }
+
+    function isFileInInternalBucket (filename: string) {
+      return internalBuckets.has(getModuleBucket(filename))
+    }
+
+    function isFileKnownToBeInternal (filename: string) {
+      return knownInternalFilenames.has(filename)
+    }
+
+    /**
+     * If we need to emit JS for a file, force TS to consider it non-external
+     */
+    const fixupResolvedModule = (resolvedModule: _ts.ResolvedModule | _ts.ResolvedTypeReferenceDirective) => {
+      const { resolvedFileName } = resolvedModule
+      if (resolvedFileName === undefined) return
+      // .ts is always switched to internal
+      // .js is switched on-demand
+      if (
+        resolvedModule.isExternalLibraryImport && (
+          (resolvedFileName.endsWith('.ts') && !resolvedFileName.endsWith('.d.ts')) ||
+          isFileKnownToBeInternal(resolvedFileName) ||
+          isFileInInternalBucket(resolvedFileName)
+        )
+      ) {
+        resolvedModule.isExternalLibraryImport = false
+      }
+      if (!resolvedModule.isExternalLibraryImport) {
+        knownInternalFilenames.add(resolvedFileName)
+      }
+    }
+    /*
+     * NOTE:
+     * Older ts versions do not pass `redirectedReference` nor `options`.
+     * We must pass `redirectedReference` to newer ts versions, but cannot rely on `options`, hence the weird argument name
+     */
+    const resolveModuleNames: _ts.LanguageServiceHost['resolveModuleNames'] = (moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: _ts.ResolvedProjectReference | undefined, optionsOnlyWithNewerTsVersions: _ts.CompilerOptions): (_ts.ResolvedModule | undefined)[] => {
+      return moduleNames.map(moduleName => {
+        const { resolvedModule } = ts.resolveModuleName(moduleName, containingFile, config.options, serviceHost, moduleResolutionCache, redirectedReference)
+        if (resolvedModule) {
+          fixupResolvedModule(resolvedModule)
+        }
+        return resolvedModule
+      })
+    }
+
+    // language service never calls this, but TS docs recommend that we implement it
+    const getResolvedModuleWithFailedLookupLocationsFromCache: _ts.LanguageServiceHost['getResolvedModuleWithFailedLookupLocationsFromCache'] = (moduleName, containingFile): _ts.ResolvedModuleWithFailedLookupLocations | undefined => {
+      const ret = ts.resolveModuleNameFromCache(moduleName, containingFile, moduleResolutionCache)
+      if (ret && ret.resolvedModule) {
+        fixupResolvedModule(ret.resolvedModule)
+      }
+      return ret
+    }
+
+    const resolveTypeReferenceDirectives: _ts.LanguageServiceHost['resolveTypeReferenceDirectives'] = (typeDirectiveNames: string[], containingFile: string, redirectedReference: _ts.ResolvedProjectReference | undefined, options: _ts.CompilerOptions): (_ts.ResolvedTypeReferenceDirective | undefined)[] => {
+      // Note: seems to be called with empty typeDirectiveNames array for all files.
+      return typeDirectiveNames.map(typeDirectiveName => {
+        const { resolvedTypeReferenceDirective } = ts.resolveTypeReferenceDirective(typeDirectiveName, containingFile, config.options, serviceHost, redirectedReference)
+        if (resolvedTypeReferenceDirective) {
+          fixupResolvedModule(resolvedTypeReferenceDirective)
+        }
+        return resolvedTypeReferenceDirective
+      })
+    }
+
+    return {
+      resolveModuleNames,
+      getResolvedModuleWithFailedLookupLocationsFromCache,
+      resolveTypeReferenceDirectives,
+      isFileKnownToBeInternal,
+      markBucketOfFilenameInternal
+    }
+  }
+
   // Use full language services when the fast option is disabled.
   if (!transpileOnly) {
     const fileContents = new Map<string, string>()
@@ -536,14 +645,15 @@ export function create (rawOptions: CreateOptions = {}): Register {
       }
 
       // Create the compiler host for type checking.
-      const serviceHost: _ts.LanguageServiceHost = {
+      const serviceHost: _ts.LanguageServiceHost & Required<Pick<_ts.LanguageServiceHost, 'fileExists' | 'readFile'>> = {
         getProjectVersion: () => String(projectVersion),
         getScriptFileNames: () => Array.from(rootFileNames),
         getScriptVersion: (fileName: string) => {
           const version = fileVersions.get(fileName)
           return version ? version.toString() : ''
         },
         getScriptSnapshot (fileName: string) {
+          // TODO ordering of this with getScriptVersion?  Should they sync up?
           let contents = fileContents.get(fileName)
 
           // Read contents into TypeScript memory cache.
@@ -563,21 +673,27 @@ export function create (rawOptions: CreateOptions = {}): Register {
         getDirectories: cachedLookup(debugFn('getDirectories', ts.sys.getDirectories)),
         fileExists: cachedLookup(debugFn('fileExists', fileExists)),
         directoryExists: cachedLookup(debugFn('directoryExists', ts.sys.directoryExists)),
+        realpath: ts.sys.realpath ? cachedLookup(debugFn('realpath', ts.sys.realpath)) : undefined,
         getNewLine: () => ts.sys.newLine,
         useCaseSensitiveFileNames: () => ts.sys.useCaseSensitiveFileNames,
         getCurrentDirectory: () => cwd,
         getCompilationSettings: () => config.options,
         getDefaultLibFileName: () => ts.getDefaultLibFilePath(config.options),
         getCustomTransformers: getCustomTransformers
       }
+      const { resolveModuleNames, getResolvedModuleWithFailedLookupLocationsFromCache, resolveTypeReferenceDirectives, isFileKnownToBeInternal, markBucketOfFilenameInternal } = createResolverFunctions(serviceHost)
+      serviceHost.resolveModuleNames = resolveModuleNames
+      serviceHost.getResolvedModuleWithFailedLookupLocationsFromCache = getResolvedModuleWithFailedLookupLocationsFromCache
+      serviceHost.resolveTypeReferenceDirectives = resolveTypeReferenceDirectives
 
       const registry = ts.createDocumentRegistry(ts.sys.useCaseSensitiveFileNames, cwd)
       const service = ts.createLanguageService(serviceHost, registry)
 
       const updateMemoryCache = (contents: string, fileName: string) => {
-        // Add to `rootFiles` if not already there
-        // This is necessary to force TS to emit output
-        if (!rootFileNames.has(fileName)) {
+        // Add to `rootFiles` as necessary, either to make TS include a file it has not seen,
+        // or to trigger a re-classification of files from external to internal.
+        if (!rootFileNames.has(fileName) && !isFileKnownToBeInternal(fileName)) {
+          markBucketOfFilenameInternal(fileName)
           rootFileNames.add(fileName)
           // Increment project version for every change to rootFileNames.
           projectVersion++
@@ -649,13 +765,15 @@ export function create (rawOptions: CreateOptions = {}): Register {
         return { name, comment }
       }
     } else {
-      const sys = {
+      const sys: _ts.System & _ts.FormatDiagnosticsHost = {
         ...ts.sys,
         ...diagnosticHost,
         readFile: (fileName: string) => {
           const cacheContents = fileContents.get(fileName)
           if (cacheContents !== undefined) return cacheContents
-          return cachedReadFile(fileName)
+          const contents = cachedReadFile(fileName)
+          if (contents) fileContents.set(fileName, contents)
+          return contents
         },
         readDirectory: ts.sys.readDirectory,
         getDirectories: cachedLookup(debugFn('getDirectories', ts.sys.getDirectories)),
@@ -678,6 +796,9 @@ export function create (rawOptions: CreateOptions = {}): Register {
           getDefaultLibFileName: () => normalizeSlashes(join(dirname(compiler), ts.getDefaultLibFileName(config.options))),
           useCaseSensitiveFileNames: () => sys.useCaseSensitiveFileNames
         }
+      const { resolveModuleNames, resolveTypeReferenceDirectives, isFileKnownToBeInternal, markBucketOfFilenameInternal } = createResolverFunctions(host)
+      host.resolveModuleNames = resolveModuleNames
+      host.resolveTypeReferenceDirectives = resolveTypeReferenceDirectives
 
       // Fallback for older TypeScript releases without incremental API.
       let builderProgram = ts.createIncrementalProgram
@@ -704,17 +825,22 @@ export function create (rawOptions: CreateOptions = {}): Register {
 
       // Set the file contents into cache manually.
       const updateMemoryCache = (contents: string, fileName: string) => {
-        const sourceFile = builderProgram.getSourceFile(fileName)
-
-        fileContents.set(fileName, contents)
+        const previousContents = fileContents.get(fileName)
+        const contentsChanged = previousContents !== contents
+        if (contentsChanged) {
+          fileContents.set(fileName, contents)
+        }
 
         // Add to `rootFiles` when discovered by compiler for the first time.
-        if (sourceFile === undefined) {
+        let addedToRootFileNames = false
+        if (!rootFileNames.has(fileName) && !isFileKnownToBeInternal(fileName)) {
+          markBucketOfFilenameInternal(fileName)
           rootFileNames.add(fileName)
+          addedToRootFileNames = true
         }
 
         // Update program when file changes.
-        if (sourceFile === undefined || sourceFile.text !== contents) {
+        if (addedToRootFileNames || contentsChanged) {
           builderProgram = ts.createEmitAndSemanticDiagnosticsBuilderProgram(
             Array.from(rootFileNames),
             config.options,

tests/from-node-modules/from-node-modules.ts

@@ -1 +1,10 @@
-import 'test'
+// These files are resolved by the typechecker
+import * as tsmie from 'external/typescript-module-imported-externally'
+import * as jsmie from 'external/javascript-module-imported-externally'
+// These files are unknown to the compiler until required.
+const tsmre = require('external/typescript-module-required-externally')
+const jsmre = require('external/javascript-module-required-externally')
+
+import * as external from 'external'
+
+console.log(JSON.stringify({external, tsmie, jsmie, tsmre, jsmre}, null, 2))