Repository navigation
Add option to fully resolve paths, making compiled code ECMAScript Module-ready - #31
Conversation
…ias and fullpath resolution
| import { Output } from './utils'; | ||
| import { Output, resolveFullImportPaths } from './utils'; | ||
|
|
||
| export interface ReplaceTscAliasPathsOptions { |
There was a problem hiding this comment.
Moved the options into an exported interface to make it accessible to API consumers.
| }; | ||
|
|
||
| const replaceAlias = (file: string): boolean => { | ||
| const replaceAlias = (file: string, resolveFullPath?: boolean): boolean => { |
There was a problem hiding this comment.
Since fullpath resolution is an overlapping problem with alias resolution, I felt like it was reasonable to add it here. It feels a bit like a side effect, but a rename of the function (e.g. to resolveImporPaths) would make it less weird.
I considered doing it separately from this function, but that would require an additional save to disk. Probably wouldn't really matter.
Also, I added the parameter to make this function more portable in case it gets moved into the outer scope at some point. If it's intended to be kept in the inner scope, the parameter could be removed and the reference changed to a direct reference to options?.resolveFullPath.
|
|
||
| // Fully resolve all import paths (not just aliased ones) | ||
| // *after* the aliases are resolved | ||
| if (resolveFullPath) { |
There was a problem hiding this comment.
This is where the new resolver gets called, if the option is supplied.
| // Separate patterns for each style of import statement, | ||
| // wrapped in non-capturing groups, | ||
| // so that they can be strung together in one big pattern. | ||
| const funcStyle = `(?:\\b(?:import|require)\\s*\\(\\s*${importString}\\s*\\))`; |
There was a problem hiding this comment.
I merged the original two-pattern import-statement regexes into one, and created a class to make repeated use of that new pattern easier.
The new pattern closely matches the original two in behavior, but adds some additional constraints.
| * Replace all source import paths, using a replacer | ||
| * function (a la `String.prototype.replace(globalRegex,replacer)`) | ||
| */ | ||
| replaceSourceImportPaths(replacer: StringReplacer) { |
There was a problem hiding this comment.
This is essentially what the code was originally doing to resolve aliases, as a method that allows the dev to not have to also provide the regex.
| * valid statements are incrementing numbers, starting from 0, | ||
| * so that it is easy to verify validity of results. | ||
| */ | ||
| const sampleImportStatements = ` |
There was a problem hiding this comment.
New test cases can easily be added here. Currently it's assumed that the valid ones will use numeric characters for the import paths, so that the valid "paths" can be automatically pulled out and tested against.
| `; | ||
|
|
||
| function runTestProject(projectNumber: number) { | ||
| const projectDir = join(projectsRoot, `project${projectNumber}`); |
There was a problem hiding this comment.
I originally pulled this out into its own function because I misunderstood how the test projects worked, and wanted to be able to run this in different contexts.
| } | ||
|
|
||
| it(`Import regex matches import statements`, () => { | ||
| const expectedImportPaths = sampleImportStatements.match( |
There was a problem hiding this comment.
This is where I use the assumption that all the valid import statements in the sample have only digits in their import strings.
| }); | ||
| }); | ||
|
|
||
| // Run tests on projects. 9-11 are for testing fullpath file resolution |
There was a problem hiding this comment.
Manual review of the resulting files for the new 9-11 projects (which use the new flag) gave the expected results. I'm not sure what the coverage of import statement cases is with these tests, but between the direct testing of the regex used and these tests to ensure that paths can be fully resolved, it should be sufficient coverage.
|
You did a great job, thanks for the detailed explanations |
Typescript configs allow for both path aliases and node-style module resolution, while not fully supporting either on compile due to not modifying import paths.
tsc-aliashandles the path aliases part of this problem, but not the overlapping module-resolution problem.When using ECMAScript modules (ESM) in Node, being able to write in Typescript without full resolution is nice but the compiled JavaScript cannot run because ESM requires full paths.
The new feature attempts to find all import paths and then attempts to resolve them to an existing JavaScript file. If found, it replaces the original with the fully-resolved path.
While this could be done as a standalone tool, being able to use a single tool to resolve path issues in tsc-compiled code would be useful.
Additional changes:
tsc-aliascall to include the new--resolve-full-pathsflag, so that there are test cases for this new feature.The 2nd commit removes the two-regex matching used by the alias resolver and replaces it with the single-regex pattern. The new pattern does not exactly match the use of the original two patterns, but the new pattern successfully matches all test cases so it should not create surprises.