Skip to content

Commit

Permalink
fix(compiler): fix issue with aliased paths getting cut off (#4481)
Browse files Browse the repository at this point in the history
This fixes an issue (STENCIL-840) which is mentioned here:
ionic-team/ionic-framework#27417 (comment)

The problem is that for _certain_ aliased paths more of the resolve
filepath is getting cut off than intended. In the example referenced in
the discussion above, for instance, the resolved path should look
something like `../native/keyboard` but instead it looks like
`../native/keyboa`.

In `rewriteAliasedSourceFileImportPaths` we rewrite aliased paths by
basically:

1. resolving the full path to the aliased module (so e.g. `@utils` is
   resolved to `src/utils/index.ts`)
2. calculating the relative path from the importing module (where there
   is an import like `import { foo } from '@utils';`). This might look
   something like `../../utils/index.ts`.
3. re-writing the resolved relative path to remove the file extension,
   giving us something like `../../utils/index`.

The problem arose in the third step. In order to get complete coverage
for all the possible file extensions we could run into we
programmatically construct a regular expression based on
`ts.Extension`. On `main` at present this looks like this:

```ts
  const extensionRegex = new RegExp(
    Object.values(ts.Extension)
      .map((extension) => `${extension}$`)
      .join('|')
  );
```

The problem is that `Object.values(ts.Extension)` looks like this:

```ts
[
  '.ts',
  '.tsx',
  '.d.ts',
  '.js',
  '.jsx',
  '.json',
  '.tsbuildinfo',
  '.mjs',
  '.mts',
  '.d.mts',
  '.cjs',
  '.cts',
  '.d.cts'
]
```

Thus our regular expression will end up having patterns in it like

```ts
/.d.ts$/
```

This will match the substring `rd.ts` at the end of a string like
`../native/keyboard.ts`, because `.` is the wildcard and will match any
character. This is not what we want!

The solution is to replace `"."` in the extensions with `"\\."` to match
only the string `"."`, which was the original intention.
  • Loading branch information
alicewriteswrongs authored Jun 22, 2023
1 parent 2b13b8a commit 1a2c160
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/compiler/transformers/rewrite-aliased-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,13 @@ function rewriteAliasedImport(
// from the import path
const extensionRegex = new RegExp(
Object.values(ts.Extension)
.map((extension) => `${extension}$`)
// The values of `ts.Extension` look like `".d.ts"`, `".ts"`, etc
//
// We want to use them to match file extensions at the end of strings,
// like `foo.ts`. In order to match on the exact file extension strings we
// need to escape periods (`"."`) so that they are correctly interpreted as
// literal characters instead of as wildcards.
.map((extension) => `${extension}$`.replace('.', '\\.'))
.join('|')
);

Expand Down
24 changes: 24 additions & 0 deletions src/compiler/transformers/test/rewrite-aliased-paths.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ async function pathTransformTranspile(component: string, inputFileName = 'module
paths: {
'@namespace': [path.join(config.rootDir, 'name/space.ts')],
'@namespace/subdir': [path.join(config.rootDir, 'name/space/subdir.ts')],
'@namespace/keyboard': [path.join(config.rootDir, 'name/space/keyboard.ts')],
},
declaration: true,
};
Expand All @@ -38,6 +39,10 @@ async function pathTransformTranspile(component: string, inputFileName = 'module
// transform the module ID
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;');
await compilerContext.fs.writeFile(
path.join(config.rootDir, 'name/space/keyboard.ts'),
'export const keyboard = "keyboard"'
);

return transpileModule(
component,
Expand Down Expand Up @@ -220,4 +225,23 @@ describe('rewrite alias module paths transform', () => {
)
);
});

// this is a regression test for STENCIL-840, the issue referenced here:
// /~https://github.com/ionic-team/ionic-framework/pull/27417#discussion_r1187779290
it("shouldn't get tripped up by regexes!", async () => {
const t = await pathTransformTranspile(`
import { foo } from "@namespace/keyboard";
export class CmpA {
render() {
return <some-cmp>{ foo("bar") }</some-cmp>
}
}
`);

expect(formatCode(t.outputText)).toBe(
formatCode(
'import { foo } from "./name/space/keyboard";export class CmpA { render() { return h("some-cmp", null, foo("bar")); }}'
)
);
});
});

0 comments on commit 1a2c160

Please sign in to comment.