Fix project argument to match tsc

bendemboski · bendemboski · commit 669af32ecb69 · 2024-03-13T16:15:53.000-07:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,32 +11,72 @@ 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<string, GlintConfig | null>();
 
+  /**
+   * 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;
 
     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<T>(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<string, unknown> = {};
   let currentPath: string | undefined = entryPath;
diff --git 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 };
