Skip to content

Commit

Permalink
Override isExternalLibraryImport as needed; re-add realpath (#970)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cspotcode authored Aug 21, 2020
1 parent 836d1f2 commit c2a2048
Show file tree
Hide file tree
Showing 28 changed files with 356 additions and 29 deletions.
63 changes: 63 additions & 0 deletions development-docs/isExternalLibraryImport.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion scripts/create-merged-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
50 changes: 39 additions & 11 deletions src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
152 changes: 139 additions & 13 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -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'

/**
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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>()
Expand All @@ -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.
Expand All @@ -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++
Expand Down Expand Up @@ -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)),
Expand All @@ -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
Expand All @@ -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,
Expand Down
11 changes: 10 additions & 1 deletion tests/from-node-modules/from-node-modules.ts
Original file line number Diff line number Diff line change
@@ -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))
8 changes: 8 additions & 0 deletions tests/from-node-modules/node_modules/external/index.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit c2a2048

Please sign in to comment.