fix(compiler): add docs and tests for path transformer

This commit adds code comments and tests for the import path TS transformer. It also reverts some temp changes in a previous commit

src/compiler/transformers/map-imports-to-path-aliases.ts

@@ -2,27 +2,32 @@ import { dirname, relative } from 'path';
 import ts from 'typescript';
 import type * as d from '../../declarations';
 
-export const mapImportsToPathAliases = ({ tsCompilerOptions }: d.Config): ts.TransformerFactory<ts.SourceFile> => {
+/**
+ * This method is responsible for replacing user-defined import path aliases ({@link https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping})
+ * with generated relative import paths during the transformation step of the TS compilation process.
+ * This action is taken to prevent issues with import paths not being transpiled at build time resulting in
+ * unknown imports in output code for some output targets (`dist-collection` for instance). Output targets that do not run through a bundler
+ * are unable to resolve imports using the aliased path names and TS intentionally does not replace resolved paths as a part of
+ * their compiler ({@link https://github.com/microsoft/TypeScript/issues/10866})
+ *
+ * @param config The Stencil configuration object.
+ * @returns A factory for creating a {@link ts.Transformer}.
+ */
+export const mapImportsToPathAliases = (config: d.Config): ts.TransformerFactory<ts.SourceFile> => {
   return (transformCtx) => {
     let dirPath: string;
     let sourceFile: string;
 
-    const compilerHost = ts.createCompilerHost(tsCompilerOptions);
+    const compilerHost = ts.createCompilerHost(config.tsCompilerOptions);
 
     const visit = (node: ts.Node): ts.VisitResult<ts.Node> => {
       if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) {
         let importPath = node.moduleSpecifier.text;
 
+        // We will ignore transforming any paths that are already relative paths or
+        // imports from external modules/packages
         if (!importPath.startsWith('.')) {
-          /**
-           * When running unit tests, the `resolvedModule` property on the returned object is always undefined.
-           * In an actual build, the modules are resolved as expected.
-           *
-           * Not sure what is causing this to fail in tests. My guess is that the `transpileModule` helper method
-           * is somehow not giving context to paths for the options supplied on a `config.tsCompilerOptions`, but I am confused
-           * as to how this differs between tests and a build.
-           */
-          const module = ts.resolveModuleName(importPath, sourceFile, tsCompilerOptions, compilerHost);
+          const module = ts.resolveModuleName(importPath, sourceFile, config.tsCompilerOptions, compilerHost);
 
           if (
             module.resolvedModule?.isExternalLibraryImport === false &&

src/compiler/transformers/test/map-imports-to-path-aliases.spec.ts

@@ -1,35 +1,97 @@
 import { mockValidatedConfig } from '@stencil/core/testing';
 import { ValidatedConfig } from '../../../internal';
 import { transpileModule } from './transpile';
+import ts, { Extension } from 'typescript';
+import { mapImportsToPathAliases } from '../map-imports-to-path-aliases';
 
 describe('mapImportsToPathAliases', () => {
   let module: ReturnType<typeof transpileModule>;
   let config: ValidatedConfig;
+  let resolveModuleNameSpy: jest.SpyInstance<
+    ReturnType<typeof ts.resolveModuleName>,
+    Parameters<typeof ts.resolveModuleName>
+  >;
 
   beforeEach(() => {
     config = mockValidatedConfig({ tsconfig: './tsconfig.json', tsCompilerOptions: {} });
+
+    resolveModuleNameSpy = jest.spyOn(ts, 'resolveModuleName');
   });
 
-  /**
-   * This test fails
-   * The `transpileModule` helper method is designed to only transpile a single module's (file's)
-   * content. However, with the solution implemented to fix module imports not getting transformed respective of
-   * their path aliases in the `tsconfig.json` file for a project, this helper cannot resolve the defined imports so
-   * the text returned from the helper keeps the original import path intact.
-   */
-  it('should replace the path alias with the generated relative path', () => {
-    config.tsCompilerOptions.paths = {
-      '@utils': ['./utils'],
-    };
-    config.tsCompilerOptions.baseUrl = '.';
+  afterEach(() => {
+    resolveModuleNameSpy.mockReset();
+  });
+
+  it('should ignore relative imports', () => {
+    resolveModuleNameSpy.mockReturnValue({
+      resolvedModule: {
+        isExternalLibraryImport: false,
+        extension: Extension.Ts,
+        resolvedFileName: 'utils.js',
+      },
+    });
     const inputText = `
-        import * as dateUtils from '@utils';
+        import * as dateUtils from "../utils";
 
         dateUtils.test();
     `;
 
-    module = transpileModule(inputText, config, null, [], []);
+    module = transpileModule(inputText, config, null, [], [mapImportsToPathAliases(config)]);
+
+    expect(module.outputText).toContain('import * as dateUtils from "../utils";');
+  });
+
+  it('should ignore external imports', () => {
+    resolveModuleNameSpy.mockReturnValue({
+      resolvedModule: {
+        isExternalLibraryImport: true,
+        extension: Extension.Ts,
+        resolvedFileName: 'utils.js',
+      },
+    });
+    const inputText = `
+        import { utils } from "@stencil/core";
+
+        utils.test();
+    `;
+
+    module = transpileModule(inputText, config, null, [], [mapImportsToPathAliases(config)]);
+
+    expect(module.outputText).toContain('import { utils } from "@stencil/core";');
+  });
+
+  it('should do nothing if there is no resolved module', () => {
+    resolveModuleNameSpy.mockReturnValue({
+      resolvedModule: undefined,
+    });
+    const inputText = `
+        import { utils } from "@utils";
+
+        utils.test();
+    `;
+
+    module = transpileModule(inputText, config, null, [], [mapImportsToPathAliases(config)]);
+
+    expect(module.outputText).toContain('import { utils } from "@utils";');
+  });
+
+  // TODO(STENCIL-223): remove spy to test actual resolution behavior
+  it('should replace the path alias with the generated relative path', () => {
+    resolveModuleNameSpy.mockReturnValue({
+      resolvedModule: {
+        isExternalLibraryImport: false,
+        extension: Extension.Ts,
+        resolvedFileName: 'utils.ts',
+      },
+    });
+    const inputText = `
+        import { utils } from "@utils";
+
+        utils.test();
+    `;
+
+    module = transpileModule(inputText, config, null, [], [mapImportsToPathAliases(config)]);
 
-    expect(module.outputText).toEqual(`import * as dateUtils from '../../utils/date'`);
+    expect(module.outputText).toContain('import { utils } from "utils";');
   });
 });

src/compiler/transformers/test/transpile.ts

@@ -5,7 +5,6 @@ import { mockBuildCtx, mockCompilerCtx, mockConfig } from '@stencil/core/testing
 import ts from 'typescript';
 import { updateModule } from '../static-to-meta/parse-static';
 import { getScriptTarget } from '../transform-utils';
-import { mapImportsToPathAliases } from '../map-imports-to-path-aliases';
 
 /**
  * Testing utility for transpiling provided string containing valid Stencil code
@@ -23,7 +22,7 @@ export function transpileModule(
   beforeTransformers: ts.TransformerFactory<ts.SourceFile>[] = [],
   afterTransformers: ts.TransformerFactory<ts.SourceFile>[] = []
 ) {
-  let options = ts.getDefaultCompilerOptions();
+  const options = ts.getDefaultCompilerOptions();
   options.isolatedModules = true;
   options.suppressOutputPathCheck = true;
   options.allowNonTsExtensions = true;
@@ -41,7 +40,7 @@ export function transpileModule(
   options.declarationDir = undefined;
   options.out = undefined;
   options.outFile = undefined;
-  options.noResolve = false;
+  options.noResolve = true;
 
   options.module = ts.ModuleKind.ESNext;
   options.target = getScriptTarget();
@@ -51,23 +50,8 @@ export function transpileModule(
   options.jsxFactory = 'h';
   options.jsxFragmentFactory = 'Fragment';
 
-  /**
-   * Override the options with the supplied compiler options on the config.
-   * This ensures that the path and baseUrl attributes are passed to the transformer correctly.
-   */
-  options = {
-    ...options,
-    ...config?.tsCompilerOptions,
-  };
-
-  /**
-   * Updating the method to transpile multiple source files just to try to mock
-   * how the TS compiler would work in a real build.
-   */
-  const sourceFiles = [
-    ts.createSourceFile('utils.tsx', 'export function test() {}', options.target, true),
-    ts.createSourceFile('module.tsx', input, options.target, true),
-  ];
+  const inputFileName = 'module.tsx';
+  const sourceFile = ts.createSourceFile(inputFileName, input, options.target);
 
   let outputText: string;
 
@@ -79,24 +63,20 @@ export function transpileModule(
   };
 
   const compilerHost: ts.CompilerHost = {
-    getSourceFile: (fileName) => sourceFiles.find((ref) => ref.fileName === fileName),
+    getSourceFile: (fileName) => (fileName === inputFileName ? sourceFile : undefined),
     writeFile: emitCallback,
     getDefaultLibFileName: () => 'lib.d.ts',
     useCaseSensitiveFileNames: () => false,
     getCanonicalFileName: (fileName) => fileName,
     getCurrentDirectory: () => '',
     getNewLine: () => '',
-    fileExists: (fileName) => !!sourceFiles.find((ref) => ref.fileName === fileName),
+    fileExists: (fileName) => fileName === inputFileName,
     readFile: () => '',
     directoryExists: () => true,
     getDirectories: () => [],
   };
 
-  const tsProgram = ts.createProgram(
-    sourceFiles.map((ref) => ref.fileName),
-    options,
-    compilerHost
-  );
+  const tsProgram = ts.createProgram([inputFileName], options, compilerHost);
   const tsTypeChecker = tsProgram.getTypeChecker();
 
   config = config || mockConfig();
@@ -114,15 +94,10 @@ export function transpileModule(
     styleImportData: 'queryparams',
   };
 
-  tsProgram.emit(undefined, emitCallback, undefined, undefined, {
+  tsProgram.emit(undefined, undefined, undefined, undefined, {
     before: [convertDecoratorsToStatic(config, buildCtx.diagnostics, tsTypeChecker), ...beforeTransformers],
     after: [
       convertStaticToMeta(config, compilerCtx, buildCtx, tsTypeChecker, null, transformOpts),
-      // Hard-coding this here until failures are resolved.
-      mapImportsToPathAliases({
-        ...config,
-        tsCompilerOptions: options,
-      }),
       ...afterTransformers,
     ],
   });

src/compiler/transpile/run-program.ts

@@ -5,7 +5,7 @@ import { generateAppTypes } from '../types/generate-app-types';
 import { getComponentsFromModules, isOutputTargetDistTypes } from '../output-targets/output-utils';
 import { loadTypeScriptDiagnostics, normalizePath } from '@utils';
 import { resolveComponentDependencies } from '../entries/resolve-component-dependencies';
-import ts from 'typescript';
+import type ts from 'typescript';
 import { updateComponentBuildConditionals } from '../app-core/app-data';
 import { updateModule } from '../transformers/static-to-meta/parse-static';
 import { updateStencilTypesImports } from '../types/stencil-types';