From 669af32ecb69e6486e6183196d065cd613fbd0f3 Mon Sep 17 00:00:00 2001 From: Ben Demboski Date: Wed, 13 Mar 2024 16:15:53 -0700 Subject: [PATCH] Fix project argument to match tsc The `--project` argument was being treated as a path to some location inside the project, rather than as either a path to a tsconfig file or a path to a folder containing a tsconfig file, which is how `tsc` implements it. This meant that there was no way to run `glint` and use a config file named anything other than `tsconfig.json`. This is a breaking change because if anybody was passing a project path that pointed deeply into the project, that will no longer work. Similarly, the `analyzeProject()` unstable API's behavior has changed, and `loadConfig` export has been replaced with `loadClosestConfig` and `loadConfigFromProject` exports. --- .../core/__tests__/config/load-config.test.ts | 132 ++++++++++++++---- packages/core/__tests__/config/loader.test.ts | 46 ++++++ packages/core/src/cli/index.ts | 7 +- packages/core/src/config/index.ts | 16 ++- packages/core/src/config/loader.ts | 65 +++++++-- packages/core/src/index.ts | 9 +- 6 files changed, 231 insertions(+), 44 deletions(-) diff --git a/packages/core/__tests__/config/load-config.test.ts b/packages/core/__tests__/config/load-config.test.ts index 637592949..ebc34896b 100644 --- a/packages/core/__tests__/config/load-config.test.ts +++ b/packages/core/__tests__/config/load-config.test.ts @@ -1,10 +1,10 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import { describe, beforeEach, afterEach, test, expect } from 'vitest'; -import { loadConfig } from '../../src/config/index.js'; +import { loadClosestConfig, loadConfigFromProject } from '../../src/config/index.js'; import { normalizePath } from '../../src/config/config.js'; -describe('Config: loadConfig', () => { +describe('Config', () => { const testDir = `${os.tmpdir()}/glint-config-test-load-config-${process.pid}`; beforeEach(() => { @@ -20,35 +20,111 @@ describe('Config: loadConfig', () => { fs.rmSync(testDir, { recursive: true, force: true }); }); - test('throws an error if no config is found', () => { - expect(() => loadConfig(testDir)).toThrow(`Unable to find Glint configuration for ${testDir}`); + describe('loadClosestConfig', () => { + test('throws an error if no config is found', () => { + expect(() => loadClosestConfig(testDir)).toThrow( + `Unable to find Glint configuration for ${testDir}` + ); + }); + + test('loads from a folder', () => { + fs.writeFileSync( + `${testDir}/tsconfig.json`, + JSON.stringify({ + glint: { + environment: './local-env', + }, + }) + ); + + let config = loadClosestConfig(`${testDir}/deeply/nested/directory`); + + expect(config.rootDir).toBe(normalizePath(testDir)); + expect(config.environment.getConfiguredTemplateTags()).toEqual({ test: {} }); + }); + + test('locates config in a parent directory', () => { + fs.mkdirSync(`${testDir}/deeply/nested/directory`, { recursive: true }); + fs.writeFileSync( + `${testDir}/tsconfig.json`, + JSON.stringify({ + glint: { + environment: 'kaboom', + checkStandaloneTemplates: false, + }, + }) + ); + fs.writeFileSync( + `${testDir}/deeply/tsconfig.json`, + JSON.stringify({ + extends: '../tsconfig.json', + glint: { + environment: '../local-env', + }, + }) + ); + + let config = loadClosestConfig(`${testDir}/deeply/nested/directory`); + + expect(config.rootDir).toBe(normalizePath(`${testDir}/deeply`)); + expect(config.environment.getConfiguredTemplateTags()).toEqual({ test: {} }); + expect(config.checkStandaloneTemplates).toBe(false); + }); }); - test('locates config in a parent directory', () => { - fs.mkdirSync(`${testDir}/deeply/nested/directory`, { recursive: true }); - fs.writeFileSync( - `${testDir}/tsconfig.json`, - JSON.stringify({ - glint: { - environment: 'kaboom', - checkStandaloneTemplates: false, - }, - }) - ); - fs.writeFileSync( - `${testDir}/deeply/tsconfig.json`, - JSON.stringify({ - extends: '../tsconfig.json', - glint: { - environment: '../local-env', - }, - }) - ); + describe('loadConfigFromProject', () => { + test('throws an error if no config is found', () => { + expect(() => loadConfigFromProject(testDir)).toThrow( + `Unable to find Glint configuration for project ${testDir}` + ); + expect(() => loadConfigFromProject(`${testDir}/tsconfig.json`)).toThrow( + `Unable to find Glint configuration for project ${testDir}` + ); + }); + + test('loads from a folder', () => { + fs.writeFileSync( + `${testDir}/tsconfig.json`, + JSON.stringify({ + glint: { + environment: './local-env', + }, + }) + ); + + expect(loadConfigFromProject(testDir).rootDir).toBe(normalizePath(testDir)); + }); + + test('loads from a file', () => { + fs.writeFileSync( + `${testDir}/tsconfig.custom.json`, + JSON.stringify({ + glint: { + environment: './local-env', + }, + }) + ); + + expect(loadConfigFromProject(`${testDir}/tsconfig.custom.json`).rootDir).toBe( + normalizePath(testDir) + ); + }); - let config = loadConfig(`${testDir}/deeply/nested/directory`); + test('does not search parent directories', () => { + fs.mkdirSync(`${testDir}/sub`, { recursive: true }); + fs.writeFileSync( + `${testDir}/tsconfig.json`, + JSON.stringify({ + glint: { + environment: 'kaboom', + checkStandaloneTemplates: false, + }, + }) + ); - expect(config.rootDir).toBe(normalizePath(`${testDir}/deeply`)); - expect(config.environment.getConfiguredTemplateTags()).toEqual({ test: {} }); - expect(config.checkStandaloneTemplates).toBe(false); + expect(() => loadConfigFromProject(`${testDir}/sub`)).toThrow( + `Unable to find Glint configuration for project ${testDir}` + ); + }); }); }); diff --git a/packages/core/__tests__/config/loader.test.ts b/packages/core/__tests__/config/loader.test.ts index c1c4ea2f6..710139d9c 100644 --- a/packages/core/__tests__/config/loader.test.ts +++ b/packages/core/__tests__/config/loader.test.ts @@ -62,8 +62,54 @@ describe('Config: loadConfig', () => { let loader = new ConfigLoader(); let configA = loader.configForFile(`${testDir}/src/a.ts`); let configB = loader.configForFile(`${testDir}/src/b.ts`); + let configC = loader.configForFile(`${testDir}/src/../src/c.ts`); expect(configA).toBe(configB); + expect(configA).toBe(configC); + }); + + test('returns config from project file path', () => { + fs.writeFileSync( + `${testDir}/tsconfig.customname.json`, + JSON.stringify({ + glint: { environment: './local-env.js' }, + }) + ); + + expect( + new ConfigLoader().configForProjectPath(`${testDir}/tsconfig.customname.json`)?.rootDir + ).toBe(normalizePath(`${testDir}`)); + }); + + test('returns config from project folder path', () => { + fs.writeFileSync( + `${testDir}/tsconfig.json`, + JSON.stringify({ + glint: { environment: './local-env.js' }, + }) + ); + + expect(new ConfigLoader().configForProjectPath(testDir)?.rootDir).toBe( + normalizePath(`${testDir}`) + ); + }); + + test('returns null for invalid project paths', () => { + fs.mkdirSync(`${testDir}/packages/a/src`, { recursive: true }); + + fs.writeFileSync( + `${testDir}/tsconfig.json`, + JSON.stringify({ + glint: { environment: './local-env.js' }, + }) + ); + + expect( + new ConfigLoader().configForProjectPath(`${testDir}/tsconfig.missing.json`) + ).toBeNull(); + expect( + new ConfigLoader().configForProjectPath(`${testDir}/packages/a/src`) + ).toBeNull(); }); describe('extending other config', () => { diff --git a/packages/core/src/cli/index.ts b/packages/core/src/cli/index.ts index 054ce3eaa..92b2c8f49 100644 --- a/packages/core/src/cli/index.ts +++ b/packages/core/src/cli/index.ts @@ -1,6 +1,6 @@ import { createRequire } from 'node:module'; import yargs from 'yargs'; -import { findTypeScript, loadConfig } from '../config/index.js'; +import { findTypeScript, loadClosestConfig, loadConfigFromProject } from '../config/index.js'; import { performWatch } from './perform-watch.js'; import { performCheck } from './perform-check.js'; import { determineOptionsToExtend } from './options.js'; @@ -18,7 +18,7 @@ const argv = yargs(process.argv.slice(2)) .option('project', { alias: 'p', string: true, - description: 'The path to the tsconfig file to use', + description: 'The path to the tsconfig file to use or the folder containing it', }) .option('watch', { alias: 'w', @@ -120,7 +120,8 @@ if (argv.build) { performBuild(ts, projects, buildOptions); } } else { - const glintConfig = loadConfig(argv.project ?? cwd); + const glintConfig = + argv.project !== undefined ? loadConfigFromProject(argv.project) : loadClosestConfig(cwd); const optionsToExtend = determineOptionsToExtend(argv); validateTSOrExit(glintConfig.ts); diff --git a/packages/core/src/config/index.ts b/packages/core/src/config/index.ts index 5d461da34..eeb399b54 100644 --- a/packages/core/src/config/index.ts +++ b/packages/core/src/config/index.ts @@ -6,12 +6,26 @@ export { GlintConfig } from './config.js'; export { GlintEnvironment } from './environment.js'; export { ConfigLoader, findTypeScript } from './loader.js'; +/** + * Loads glint configuration from the specified project path. If a path to a + * file is passed, the config is loaded from that file. If the path to a folder + * is passed, the config is loaded from the `tsconfig.json` or `jsconfig.json` + * file contained in that folder. Raises an error if no configuration is found. + */ +export function loadConfigFromProject(from: string): GlintConfig { + let config = new ConfigLoader().configForProjectPath(from); + if (!config) { + throw new SilentError(`Unable to find Glint configuration for project ${from}`); + } + return config; +} + /** * Loads glint configuration, starting from the given directory * and searching upwards and raising an error if no configuration * is found. */ -export function loadConfig(from: string): GlintConfig { +export function loadClosestConfig(from: string): GlintConfig { let config = findConfig(from); if (!config) { throw new SilentError(`Unable to find Glint configuration for ${from}`); diff --git a/packages/core/src/config/loader.ts b/packages/core/src/config/loader.ts index 5624fc938..45f023ce2 100644 --- a/packages/core/src/config/loader.ts +++ b/packages/core/src/config/loader.ts @@ -1,5 +1,6 @@ import { createRequire } from 'node:module'; import * as path from 'node:path'; +import * as fs from 'node:fs'; import SilentError from 'silent-error'; import { GlintConfig } from './config.js'; import { GlintConfigInput } from '@glint/core/config-types'; @@ -10,18 +11,48 @@ const require = createRequire(import.meta.url); type TypeScript = typeof TS; /** - * `ConfigLoader` provides an interface for finding the Glint config that - * applies to a given file or directory, ensuring that only a single instance - * of `GlintConfig` is ever created for a given `tsconfig.json` or - * `jsconfig.json` source file. + * `ConfigLoader` provides an interface for finding and loading GLint + * configurations from config files (e.g. `tsconfig.json` or `jsconfig.json`), + * and ensuring that only a single instance of `GlintConfig` is ever created for + * a given config file. */ export class ConfigLoader { private configs = new Map(); + /** + * Given the path to a configuration file, or to a folder containing a + * `tsconfig.json` or `jsconfig.json`, load the configuration. This is meant + * to implement the behavior of `glint`/`tsc`'s `--project` command-line + * option. + */ + public configForProjectPath(configPath: string): GlintConfig | null { + let tsConfigPath = path.join(configPath, 'tsconfig.json'); + let jsConfigPath = path.join(configPath, 'tsconfig.json'); + + if (fileExists(configPath)) { + return this.configForConfigFile(configPath); + } else if (fileExists(tsConfigPath)) { + return this.configForConfigFile(tsConfigPath); + } else if (fileExists(jsConfigPath)) { + return this.configForConfigFile(jsConfigPath); + } else { + return null; + } + } + + /** + * Given the path to a file, find the closest `tsconfig.json` or + * `jsconfig.json` file in the directory structure and load its configuration. + */ public configForFile(filePath: string): GlintConfig | null { return this.configForDirectory(path.dirname(filePath)); } + /** + * Give the path to a directory, find the closest `tsconfig.json` or + * `jsconfig.json` file in the directory structure, including in the directory + * itself, and load its configuration. + */ public configForDirectory(directory: string): GlintConfig | null { let ts = findTypeScript(directory); if (!ts) return null; @@ -29,13 +60,23 @@ export class ConfigLoader { let configPath = findNearestConfigFile(ts, directory); if (!configPath) return null; - let existing = this.configs.get(configPath); + return this.configForConfigFile(configPath, ts); + } + + private configForConfigFile(configPath: string, tsArg?: TypeScript): GlintConfig | null { + let ts = tsArg || findTypeScript(path.dirname(configPath)); + if (!ts) return null; + + // Normalize the config path + let absPath = path.resolve(configPath); + + let existing = this.configs.get(absPath); if (existing !== undefined) return existing; - let configInput = loadConfigInput(ts, configPath); - let config = configInput ? new GlintConfig(ts, configPath, configInput) : null; + let configInput = loadConfigInput(ts, absPath); + let config = configInput ? new GlintConfig(ts, absPath, configInput) : null; - this.configs.set(configPath, config); + this.configs.set(absPath, config); return config; } @@ -61,6 +102,14 @@ function tryResolve(load: () => T): T | null { } } +function fileExists(filePath: string): boolean { + try { + return fs.statSync(filePath).isFile(); + } catch (e) { + return false; + } +} + function loadConfigInput(ts: TypeScript, entryPath: string): GlintConfigInput | null { let fullGlintConfig: Record = {}; let currentPath: string | undefined = entryPath; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7b6cd4685..120823ee4 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,4 +1,4 @@ -import { GlintConfig, loadConfig } from './config/index.js'; +import { GlintConfig, loadClosestConfig, loadConfigFromProject } from './config/index.js'; import DocumentCache from './common/document-cache.js'; import TransformManager from './common/transform-manager.js'; import GlintLanguageServer from './language-server/glint-language-server.js'; @@ -25,8 +25,9 @@ export const pathUtils = utils; * * @internal */ -export function analyzeProject(projectDirectory: string = process.cwd()): ProjectAnalysis { - let glintConfig = loadConfig(projectDirectory); +export function analyzeProject(from?: string): ProjectAnalysis { + let glintConfig = + from !== undefined ? loadConfigFromProject(from) : loadClosestConfig(process.cwd()); let documents = new DocumentCache(glintConfig); let transformManager = new TransformManager(glintConfig, documents); let languageServer = new GlintLanguageServer(glintConfig, documents, transformManager); @@ -40,6 +41,6 @@ export function analyzeProject(projectDirectory: string = process.cwd()): Projec }; } -export { loadConfig }; +export { loadClosestConfig, loadConfigFromProject }; export type { TransformManager, GlintConfig, GlintLanguageServer };