From 9bd05a27a3d72e6b75a92b479a0298e198e621a1 Mon Sep 17 00:00:00 2001 From: Dan Freeman Date: Sun, 24 Sep 2023 18:34:34 +0200 Subject: [PATCH 1/4] Add `resolveModuleNameLiterals` to `TransformManager` --- packages/core/src/common/transform-manager.ts | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/core/src/common/transform-manager.ts b/packages/core/src/common/transform-manager.ts index 9530aec46..fd849f0e3 100644 --- a/packages/core/src/common/transform-manager.ts +++ b/packages/core/src/common/transform-manager.ts @@ -21,13 +21,25 @@ type TransformInfo = { export default class TransformManager { private transformCache = new Map(); + private moduleResolutionHost: ts.ModuleResolutionHost; private readonly ts: typeof import('typescript'); + public readonly moduleResolutionCache: ts.ModuleResolutionCache; + constructor( private glintConfig: GlintConfig, private documents: DocumentCache = new DocumentCache(glintConfig) ) { this.ts = glintConfig.ts; + this.moduleResolutionCache = this.ts.createModuleResolutionCache( + this.ts.sys.getCurrentDirectory(), + (name) => name + ); + this.moduleResolutionHost = { + ...this.ts.sys, + readFile: this.readTransformedFile, + fileExists: this.fileExists, + }; } public getTransformDiagnostics(fileName?: string): Array { @@ -161,6 +173,32 @@ export default class TransformManager { return { transformedFileName, transformedOffset }; } + public resolveModuleNameLiterals = ( + moduleLiterals: readonly ts.StringLiteralLike[], + containingFile: string, + redirectedReference: ts.ResolvedProjectReference | undefined, + options: ts.CompilerOptions + ): readonly ts.ResolvedModuleWithFailedLookupLocations[] => { + return moduleLiterals.map((literal) => { + // If import paths are allowed to include TS extensions (`.ts`, `.tsx`, etc), then we want to + // ensure we normalize things like `.gts` to the standard script path we present elsewhere so + // that TS understands the intent. + // @ts-ignore: this flag isn't available in the oldest versions of TS we support + let scriptPath = options.allowImportingTsExtensions + ? this.getScriptPathForTS(literal.text) + : literal.text; + + return this.ts.resolveModuleName( + scriptPath, + containingFile, + options, + this.moduleResolutionHost, + this.moduleResolutionCache, + redirectedReference + ); + }); + }; + public watchTransformedFile = ( path: string, originalCallback: ts.FileWatcherCallback, @@ -175,6 +213,8 @@ export default class TransformManager { let { glintConfig, documents } = this; let callback: ts.FileWatcherCallback = (watchedPath, eventKind) => { if (eventKind === this.ts.FileWatcherEventKind.Deleted) { + // Adding or removing a file invalidates most of what we think we know about module resolution + this.moduleResolutionCache.clear(); this.documents.removeDocument(watchedPath); } else { this.documents.markDocumentStale(watchedPath); @@ -213,8 +253,11 @@ export default class TransformManager { throw new Error('Internal error: TS `watchDirectory` unavailable'); } - let callback: ts.DirectoryWatcherCallback = (filename) => + let callback: ts.DirectoryWatcherCallback = (filename) => { + // Adding or removing a file invalidates most of what we think we know about module resolution + this.moduleResolutionCache.clear(); originalCallback(this.getScriptPathForTS(filename)); + }; return this.ts.sys.watchDirectory(path, callback, recursive, options); }; From 7e41f45d4ac64fbf3ebd81e716acbc0e94422083 Mon Sep 17 00:00:00 2001 From: Dan Freeman Date: Sun, 24 Sep 2023 18:35:23 +0200 Subject: [PATCH 2/4] Wire module resolution updates into the CLI --- .../__tests__/cli/custom-extensions.test.ts | 45 +++++++++++++++++++ packages/core/src/cli/perform-build-watch.ts | 5 ++- packages/core/src/cli/perform-build.ts | 5 ++- packages/core/src/cli/perform-check.ts | 2 + packages/core/src/cli/perform-watch.ts | 3 ++ .../src/cli/utils/transform-manager-pool.ts | 34 +++++++++++++- 6 files changed, 90 insertions(+), 4 deletions(-) diff --git a/packages/core/__tests__/cli/custom-extensions.test.ts b/packages/core/__tests__/cli/custom-extensions.test.ts index 56a74db52..ec01eabd1 100644 --- a/packages/core/__tests__/cli/custom-extensions.test.ts +++ b/packages/core/__tests__/cli/custom-extensions.test.ts @@ -3,6 +3,8 @@ import { stripIndent } from 'common-tags'; import stripAnsi = require('strip-ansi'); import { describe, beforeEach, afterEach, test, expect } from 'vitest'; import { Project } from 'glint-monorepo-test-utils'; +import typescript from 'typescript'; +import semver from 'semver'; describe('CLI: custom extensions', () => { let project!: Project; @@ -121,4 +123,47 @@ describe('CLI: custom extensions', () => { await watch.terminate(); }); }); + + describe('module resolution with explicit extensions', () => { + beforeEach(() => { + project.setGlintConfig({ environment: 'ember-template-imports' }); + project.write({ + 'index.gts': stripIndent` + import Greeting from './Greeting.gts'; + + `, + 'Greeting.gts': stripIndent` + + `, + }); + }); + + test('is illegal by default', async () => { + let result = await project.check({ reject: false }); + + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.stderr)).toMatchInlineSnapshot(` + "index.gts:1:22 - error TS2307: Cannot find module './Greeting.gts' or its corresponding type declarations. + + 1 import Greeting from './Greeting.gts'; + ~~~~~~~~~~~~~~~~ + " + `); + }); + + test.runIf(semver.gte(typescript.version, '5.0.0'))( + 'works with `allowImportingTsExtensions: true`', + async () => { + project.updateTsconfig((config) => { + config.compilerOptions ??= {}; + config.compilerOptions['allowImportingTsExtensions'] = true; + }); + + let result = await project.check(); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(''); + } + ); + }); }); diff --git a/packages/core/src/cli/perform-build-watch.ts b/packages/core/src/cli/perform-build-watch.ts index 7c6f5b92d..8444ecc7a 100644 --- a/packages/core/src/cli/perform-build-watch.ts +++ b/packages/core/src/cli/perform-build-watch.ts @@ -10,7 +10,7 @@ export function performBuildWatch( projects: string[], buildOptions: TS.BuildOptions ): void { - let transformManagerPool = new TransformManagerPool(ts.sys); + let transformManagerPool = new TransformManagerPool(ts); let formatDiagnostic = buildDiagnosticFormatter(ts); let buildProgram = ts.createEmitAndSemanticDiagnosticsBuilderProgram; @@ -20,6 +20,9 @@ export function performBuildWatch( (diagnostic) => console.error(formatDiagnostic(diagnostic)) ); + // @ts-ignore: This hook was added in TS5, and is safely irrelevant in earlier versions. Once we drop support for 4.x, we can also remove this @ts-ignore comment. + host.resolveModuleNameLiterals = transformManagerPool.resolveModuleNameLiterals; + let builder = ts.createSolutionBuilderWithWatch(host, projects, buildOptions); builder.build(); } diff --git a/packages/core/src/cli/perform-build.ts b/packages/core/src/cli/perform-build.ts index 52b91bd36..9b5ac3379 100644 --- a/packages/core/src/cli/perform-build.ts +++ b/packages/core/src/cli/perform-build.ts @@ -13,7 +13,7 @@ interface BuildOptions extends TS.BuildOptions { } export function performBuild(ts: TypeScript, projects: string[], buildOptions: BuildOptions): void { - let transformManagerPool = new TransformManagerPool(ts.sys); + let transformManagerPool = new TransformManagerPool(ts); let formatDiagnostic = buildDiagnosticFormatter(ts); let buildProgram = ts.createEmitAndSemanticDiagnosticsBuilderProgram; @@ -23,6 +23,9 @@ export function performBuild(ts: TypeScript, projects: string[], buildOptions: B (diagnostic) => console.error(formatDiagnostic(diagnostic)) ); + // @ts-ignore: This hook was added in TS5, and is safely irrelevant in earlier versions. Once we drop support for 4.x, we can also remove this @ts-ignore comment. + host.resolveModuleNameLiterals = transformManagerPool.resolveModuleNameLiterals; + let builder = ts.createSolutionBuilder(host, projects, buildOptions); let exitStatus = buildOptions.clean ? builder.clean() : builder.build(); process.exit(exitStatus); diff --git a/packages/core/src/cli/perform-check.ts b/packages/core/src/cli/perform-check.ts index 58c026053..3de364b76 100644 --- a/packages/core/src/cli/perform-check.ts +++ b/packages/core/src/cli/perform-check.ts @@ -63,6 +63,8 @@ function createCompilerHost( ? ts.createIncrementalCompilerHost(options, sysForCompilerHost(ts, transformManager)) : ts.createCompilerHost(options); + // @ts-ignore: This hook was added in TS5, and is safely irrelevant in earlier versions. Once we drop support for 4.x, we can also remove this @ts-ignore comment. + host.resolveModuleNameLiterals = transformManager.resolveModuleNameLiterals; host.fileExists = transformManager.fileExists; host.readFile = transformManager.readTransformedFile; host.readDirectory = transformManager.readDirectory; diff --git a/packages/core/src/cli/perform-watch.ts b/packages/core/src/cli/perform-watch.ts index 3a2b5dd0e..36608f5d1 100644 --- a/packages/core/src/cli/perform-watch.ts +++ b/packages/core/src/cli/perform-watch.ts @@ -19,5 +19,8 @@ export function performWatch(glintConfig: GlintConfig, optionsToExtend: ts.Compi (diagnostic) => console.error(formatDiagnostic(diagnostic)) ); + // @ts-ignore: This hook was added in TS5, and is safely irrelevant in earlier versions. Once we drop support for 4.x, we can also remove this @ts-ignore comment. + host.resolveModuleNameLiterals = transformManager.resolveModuleNameLiterals; + ts.createWatchProgram(host); } diff --git a/packages/core/src/cli/utils/transform-manager-pool.ts b/packages/core/src/cli/utils/transform-manager-pool.ts index 035f0652e..5aa6f769e 100644 --- a/packages/core/src/cli/utils/transform-manager-pool.ts +++ b/packages/core/src/cli/utils/transform-manager-pool.ts @@ -17,6 +17,7 @@ import { assert } from './assert.js'; * config. */ export default class TransformManagerPool { + #rootTS: typeof TS; #rootSys: TS.System; #managers = new Map(); #loader = new ConfigLoader(); @@ -25,8 +26,9 @@ export default class TransformManagerPool { return true; } - constructor(sys: TS.System) { - this.#rootSys = sys; + constructor(ts: typeof TS) { + this.#rootTS = ts; + this.#rootSys = ts.sys; } public managerForFile(path: string): TransformManager | null { @@ -45,6 +47,34 @@ export default class TransformManagerPool { return manager; } + public resolveModuleNameLiterals = ( + moduleLiterals: readonly TS.StringLiteralLike[], + containingFile: string, + redirectedReference: TS.ResolvedProjectReference | undefined, + options: TS.CompilerOptions + ): readonly TS.ResolvedModuleWithFailedLookupLocations[] => { + let resolveModuleNameLiterals = this.managerForFile(containingFile)?.resolveModuleNameLiterals; + if (resolveModuleNameLiterals) { + return resolveModuleNameLiterals( + moduleLiterals, + containingFile, + redirectedReference, + options + ); + } else { + return moduleLiterals.map((literal) => + this.#rootTS.resolveModuleName( + literal.text, + containingFile, + options, + this.#rootSys, + undefined, + redirectedReference + ) + ); + } + }; + public readDirectory = ( rootDir: string, extensions: ReadonlyArray, From ec5ea1c482682182de59bf758ebfc3fc74a8dad5 Mon Sep 17 00:00:00 2001 From: Dan Freeman Date: Sun, 24 Sep 2023 18:35:38 +0200 Subject: [PATCH 3/4] Wire module resolution updates into the language server --- .../language-server/custom-extensions.test.ts | 57 +++++++++++++++++++ .../language-server/glint-language-server.ts | 8 +++ 2 files changed, 65 insertions(+) diff --git a/packages/core/__tests__/language-server/custom-extensions.test.ts b/packages/core/__tests__/language-server/custom-extensions.test.ts index 50c3285b1..30b5b1aac 100644 --- a/packages/core/__tests__/language-server/custom-extensions.test.ts +++ b/packages/core/__tests__/language-server/custom-extensions.test.ts @@ -1,6 +1,8 @@ import { Project } from 'glint-monorepo-test-utils'; import { describe, beforeEach, afterEach, test, expect } from 'vitest'; import { stripIndent } from 'common-tags'; +import typescript from 'typescript'; +import semver from 'semver'; describe('Language Server: custom file extensions', () => { let project!: Project; @@ -254,4 +256,59 @@ describe('Language Server: custom file extensions', () => { ]); }); }); + + describe('module resolution with explicit extensions', () => { + beforeEach(() => { + project.setGlintConfig({ environment: 'ember-template-imports' }); + project.write({ + 'index.gts': stripIndent` + import Greeting from './Greeting.gts'; + + `, + 'Greeting.gts': stripIndent` + + `, + }); + }); + + test('is illegal by default', async () => { + let server = project.startLanguageServer(); + + expect(server.getDiagnostics(project.fileURI('index.gts'))).toMatchInlineSnapshot(` + [ + { + "code": 2307, + "message": "Cannot find module './Greeting.gts' or its corresponding type declarations.", + "range": { + "end": { + "character": 37, + "line": 0, + }, + "start": { + "character": 21, + "line": 0, + }, + }, + "severity": 1, + "source": "glint", + "tags": [], + }, + ] + `); + }); + + test.runIf(semver.gte(typescript.version, '5.0.0'))( + 'works with `allowImportingTsExtensions: true`', + async () => { + project.updateTsconfig((config) => { + config.compilerOptions ??= {}; + config.compilerOptions['allowImportingTsExtensions'] = true; + }); + + let server = project.startLanguageServer(); + + expect(server.getDiagnostics(project.fileURI('index.gts'))).toEqual([]); + } + ); + }); }); diff --git a/packages/core/src/language-server/glint-language-server.ts b/packages/core/src/language-server/glint-language-server.ts index a1b37d945..038662e3d 100644 --- a/packages/core/src/language-server/glint-language-server.ts +++ b/packages/core/src/language-server/glint-language-server.ts @@ -77,6 +77,8 @@ export default class GlintLanguageServer { fileExists: this.transformManager.fileExists, readFile: this.transformManager.readTransformedFile, readDirectory: this.transformManager.readDirectory, + // @ts-ignore: This hook was added in TS5, and is safely irrelevant in earlier versions. Once we drop support for 4.x, we can also remove this @ts-ignore comment. + resolveModuleNameLiterals: this.transformManager.resolveModuleNameLiterals, getCompilationSettings: () => parsedConfig.options, // Yes, this looks like a mismatch, but built-in lib declarations don't resolve // correctly otherwise, and this is what the TS wiki uses in their code snippet. @@ -119,6 +121,9 @@ export default class GlintLanguageServer { if (filePath.startsWith(this.glintConfig.rootDir)) { this.rootFileNames.add(this.transformManager.getScriptPathForTS(filePath)); } + + // Adding or removing a file invalidates most of what we think we know about module resolution. + this.transformManager.moduleResolutionCache.clear(); } public watchedFileDidChange(uri: string): void { @@ -136,6 +141,9 @@ export default class GlintLanguageServer { if (!companionPath || this.glintConfig.getSynthesizedScriptPathForTS(companionPath) !== path) { this.rootFileNames.delete(this.glintConfig.getSynthesizedScriptPathForTS(path)); } + + // Adding or removing a file invalidates most of what we think we know about module resolution. + this.transformManager.moduleResolutionCache.clear(); } public getDiagnostics(uri: string): Array { From 25a2bef2ad9044aa8108920b85496589f021f0d0 Mon Sep 17 00:00:00 2001 From: Dan Freeman Date: Sun, 24 Sep 2023 19:02:37 +0200 Subject: [PATCH 4/4] Drop unnecessary (and now breaking) old-style template declarations --- test-packages/ts-ember-app/types/global.d.ts | 6 ------ test-packages/ts-ember-preview-types/types/global.d.ts | 6 ------ 2 files changed, 12 deletions(-) delete mode 100644 test-packages/ts-ember-app/types/global.d.ts delete mode 100644 test-packages/ts-ember-preview-types/types/global.d.ts diff --git a/test-packages/ts-ember-app/types/global.d.ts b/test-packages/ts-ember-app/types/global.d.ts deleted file mode 100644 index e9a022fef..000000000 --- a/test-packages/ts-ember-app/types/global.d.ts +++ /dev/null @@ -1,6 +0,0 @@ -// Types for compiled templates -declare module 'ts-ember-app/templates/*' { - import { TemplateFactory } from 'htmlbars-inline-precompile'; - const tmpl: TemplateFactory; - export default tmpl; -} diff --git a/test-packages/ts-ember-preview-types/types/global.d.ts b/test-packages/ts-ember-preview-types/types/global.d.ts deleted file mode 100644 index e9a022fef..000000000 --- a/test-packages/ts-ember-preview-types/types/global.d.ts +++ /dev/null @@ -1,6 +0,0 @@ -// Types for compiled templates -declare module 'ts-ember-app/templates/*' { - import { TemplateFactory } from 'htmlbars-inline-precompile'; - const tmpl: TemplateFactory; - export default tmpl; -}