feat(testing): add support for transforming path aliases in spec tests (#4090)

This adds support for transforming path aliases in spec tests, using the
transformer which was added in #4042. Adding such support also revealed
a bug with the existing path transformer, notably that it did not
produce a valid path for sibling modules. See the diff for a comment
explaining the details.

Author: alicewriteswrongs

src/compiler/sys/typescript/typescript-sys.ts

@@ -55,8 +55,15 @@ export const patchTsSystemFileSystem = (
   };
 
   tsSys.directoryExists = (p) => {
-    const s = inMemoryFs.statSync(p);
-    return s.isDirectory;
+    // At present the typing for `inMemoryFs` in this function is not accurate
+    // TODO(STENCIL-728): fix typing of `inMemoryFs` parameter in `patchTypescript`, related functions
+    if (inMemoryFs) {
+      const s = inMemoryFs.statSync(p);
+      return s.isDirectory;
+    } else {
+      const s = compilerSys.statSync(p);
+      return s.isDirectory;
+    }
   };
 
   tsSys.exit = compilerSys.exit;
@@ -68,8 +75,15 @@ export const patchTsSystemFileSystem = (
       filePath = getTypescriptPathFromUrl(config, tsSys.getExecutingFilePath(), p);
     }
 
-    const s = inMemoryFs.statSync(filePath);
-    return !!(s && s.isFile);
+    // At present the typing for `inMemoryFs` in this function is not accurate
+    // TODO(STENCIL-728): fix typing of `inMemoryFs` parameter in `patchTypescript`, related functions
+    if (inMemoryFs) {
+      const s = inMemoryFs.statSync(filePath);
+      return !!(s && s.isFile);
+    } else {
+      const s = compilerSys.statSync(filePath);
+      return !!(s && s.isFile);
+    }
   };
 
   tsSys.getCurrentDirectory = compilerSys.getCurrentDirectory;
@@ -79,8 +93,15 @@ export const patchTsSystemFileSystem = (
   tsSys.getDirectories = (p) => {
     const items = compilerSys.readDirSync(p);
     return items.filter((itemPath) => {
-      const s = inMemoryFs.statSync(itemPath);
-      return !!(s && s.exists && s.isDirectory);
+      // At present the typing for `inMemoryFs` in this function is not accurate
+      // TODO(STENCIL-728): fix typing of `inMemoryFs` parameter in `patchTypescript`, related functions
+      if (inMemoryFs) {
+        const s = inMemoryFs.statSync(itemPath);
+        return !!(s && s.exists && s.isDirectory);
+      } else {
+        const s = compilerSys.statSync(itemPath);
+        return !!(s && s.isDirectory);
+      }
     });
   };
 
@@ -108,7 +129,11 @@ export const patchTsSystemFileSystem = (
       filePath = getTypescriptPathFromUrl(config, tsSys.getExecutingFilePath(), p);
     }
 
-    let content = inMemoryFs.readFileSync(filePath, { useCache: isUrl });
+    // At present the typing for `inMemoryFs` in this function is not accurate
+    // TODO(STENCIL-728): fix typing of `inMemoryFs` parameter in `patchTypescript`, related functions
+    let content = inMemoryFs
+      ? inMemoryFs.readFileSync(filePath, { useCache: isUrl })
+      : compilerSys.readFileSync(filePath);
 
     if (typeof content !== 'string' && isUrl) {
       if (IS_WEB_WORKER_ENV) {
@@ -124,7 +149,9 @@ export const patchTsSystemFileSystem = (
     return content;
   };
 
-  tsSys.writeFile = (p, data) => inMemoryFs.writeFile(p, data);
+  // At present the typing for `inMemoryFs` in this function is not accurate
+  // TODO(STENCIL-728): fix typing of `inMemoryFs` parameter in `patchTypescript`, related functions
+  tsSys.writeFile = (p, data) => (inMemoryFs ? inMemoryFs.writeFile(p, data) : compilerSys.writeFile(p, data));
 
   return tsSys;
 };

src/compiler/transformers/rewrite-aliased-paths.ts

@@ -167,7 +167,41 @@ function rewriteAliasedImport(
     node,
     retrieveTsModifiers(node),
     node.importClause,
-    transformCtx.factory.createStringLiteral(importPath),
+    transformCtx.factory.createStringLiteral(
+      // if the importee is a sibling file of the importer then `relative` will
+      // produce a somewhat confusing result. We use `dirname` to get the
+      // directory of the importer, so for example, assume we have two files
+      // `foo/bar.ts` and `foo/baz.ts` like so:
+      //
+      // ```
+      // foo
+      // ├── bar.ts
+      // └── baz.ts
+      // ```
+      //
+      // then if `baz.ts` imports a symbol from `bar.ts` we'll call
+      // `relative(fromdir, to)` like so:
+      //
+      // ```ts
+      // relative(dirname("foo/baz.ts"), "foo/bar.ts")
+      // // equivalently
+      // relative("foo", "foo/bar.ts")
+      // ```
+      //
+      // you'd think that in this case `relative` would return `'./bar.ts'` as
+      // a correct relative path to `bar.ts` from the `foo` directory, but
+      // actually in this case it returns just `bar.ts`. So since when updating
+      // import paths we're only concerned with `paths` aliases that should be
+      // transformed to relative imports anyway, we check to see if the new
+      // `importPath` starts with `'.'`, and add `'./'` if it doesn't, since
+      // otherwise Node will interpret `bar` as a module name, not a relative
+      // path.
+      //
+      // Note also that any relative paths as module specifiers which _don't_
+      // need to be transformed (e.g. `'./foo'`) have already been handled
+      // above.
+      importPath.startsWith('.') ? importPath : './' + importPath
+    ),
     node.assertClause
   );
 }

src/compiler/transformers/test/rewrite-aliased-paths.spec.ts

@@ -15,9 +15,10 @@ import { transpileModule } from './transpile';
  * transpiles the provided code.
  *
  * @param component the string of a component
+ * @param inputFileName an optional filename to use for the input file
  * @returns the tranpiled module
  */
-async function pathTransformTranspile(component: string) {
+async function pathTransformTranspile(component: string, inputFileName = 'module.tsx') {
   const compilerContext: CompilerCtx = mockCompilerCtx();
   const config = mockValidatedConfig();
 
@@ -37,8 +38,6 @@ async function pathTransformTranspile(component: string) {
   await compilerContext.fs.writeFile(path.join(config.rootDir, 'name/space.ts'), 'export const foo = x => x');
   await compilerContext.fs.writeFile(path.join(config.rootDir, 'name/space/subdir.ts'), 'export const bar = x => x;');
 
-  const inputFileName = normalizePath(path.join(config.rootDir, 'module.tsx'));
-
   return transpileModule(
     component,
     null,
@@ -47,7 +46,7 @@ async function pathTransformTranspile(component: string) {
     [],
     [rewriteAliasedDTSImportPaths],
     mockPathsConfig,
-    inputFileName
+    normalizePath(path.join(config.rootDir, inputFileName))
   );
 }
 
@@ -144,4 +143,63 @@ describe('rewrite alias module paths transform', () => {
       'import { Foo } from "./name/space/subdir";import { Bar } from "./name/space";export declare function fooUtil(foo: Foo): Bar;'
     );
   });
+
+  it('should correctly rewrite sibling paths', async () => {
+    const t = await pathTransformTranspile(
+      `
+      import { foo } from "@namespace";
+      export class CmpA {
+        render() {
+          return <some-cmp>{ foo("bar") }</some-cmp>
+        }
+      }
+    `,
+      'name/component.tsx'
+    );
+
+    // with the import filename passed to `pathTransformTranspile` the file
+    // layout during this test looks like this:
+    //
+    // ```
+    // name
+    // ├── space.ts
+    // └── component.tsx
+    // ```
+    //
+    // we need to test that the relative path from `name/component.tsx` to
+    // `name/space.ts` is resolved correctly as `'./space'`.
+    expect(t.outputText).toBe(
+      'import { foo } from "./space";export class CmpA { render() { return h("some-cmp", null, foo("bar")); }}'
+    );
+  });
+
+  it('should correctly rewrite nested sibling paths', async () => {
+    const t = await pathTransformTranspile(
+      `
+      import { foo } from "@namespace/subdir";
+      export class CmpA {
+        render() {
+          return <some-cmp>{ foo("bar") }</some-cmp>
+        }
+      }
+    `,
+      'name/component.tsx'
+    );
+
+    // with the import filename passed to `pathTransformTranspile` the file
+    // layout during this test looks like this:
+    //
+    // ```
+    // name
+    // ├── component.tsx
+    // └── space
+    //     └── subdir.ts
+    // ```
+    //
+    // we need to test that the relative path from `name/component.tsx` to
+    // `name/space/subdir.ts` is resolved correctly as `'./space/subdir'`.
+    expect(t.outputText).toBe(
+      'import { foo } from "./space/subdir";export class CmpA { render() { return h("some-cmp", null, foo("bar")); }}'
+    );
+  });
 });

src/declarations/stencil-private.ts

@@ -2299,6 +2299,11 @@ export interface E2EProcessEnv {
   __STENCIL_PUPPETEER_MODULE__?: string;
   __STENCIL_PUPPETEER_VERSION__?: number;
   __STENCIL_DEFAULT_TIMEOUT__?: string;
+
+  /**
+   * Property for injecting transformAliasedImportPaths into the Jest context
+   */
+  __STENCIL_TRANSPILE_PATHS__?: 'true' | 'false';
 }
 
 export interface AnyHTMLElement extends HTMLElement {

src/testing/jest/jest-runner.ts

@@ -13,6 +13,7 @@ export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
     const emulateConfigs = getEmulateConfigs(config.testing, config.flags);
     env.__STENCIL_EMULATE_CONFIGS__ = JSON.stringify(emulateConfigs);
     env.__STENCIL_ENV__ = JSON.stringify(config.env);
+    env.__STENCIL_TRANSPILE_PATHS__ = config.transformAliasedImportPaths ? 'true' : 'false';
 
     if (config.flags.ci || config.flags.e2e) {
       env.__STENCIL_DEFAULT_TIMEOUT__ = '30000';

src/testing/test-transpile.ts

@@ -15,6 +15,7 @@ export function transpile(input: string, opts: TranspileOptions = {}): Transpile
     style: null,
     styleImportData: 'queryparams',
     target: 'es2015', // default to es2015
+    transformAliasedImportPaths: parseStencilTranspilePaths(process.env.__STENCIL_TRANSPILE_PATHS__),
   };
 
   try {
@@ -27,3 +28,13 @@ export function transpile(input: string, opts: TranspileOptions = {}): Transpile
 
   return transpileSync(input, opts);
 }
+
+/**
+ * Turn a value which we assert can be 'true' or 'false' to a boolean.
+ *
+ * @param stencilTranspilePaths a value to 'parse'
+ * @returns a boolean
+ */
+function parseStencilTranspilePaths(stencilTranspilePaths: string): boolean {
+  return stencilTranspilePaths === 'true' ? true : false;
+}