fix(paths): Refactor Paths handling #80
Conversation
| @@ -89,7 +85,7 @@ export async function resolve( | |||
| } | |||
|
|
|||
| // Let Node.js handle all other specifiers. | |||
| return defaultResolve(specifier, context, defaultResolve); | |||
| return defaultResolve(specifier, { ...context, parentURL }, defaultResolve); | |||
There was a problem hiding this comment.
Although there isn't anything else on context on most Node.JS versions, I want to future proof this for the chance that there is/will be extra data in context.
| specifier = `./${relative( | ||
| dirname(fileURLToPath(parentURL)), | ||
| tsResolvedPath, | ||
| )}`; |
There was a problem hiding this comment.
Annoyingly I can't find a way to get relative to not return Utils/Math when I want it to be ./Utils/Math in the example tests.
There was a problem hiding this comment.
So even though this would/could/does return stuff like ./../../Utils/Math the ./../OTHER_PATH is a proper path.
| (relativePathRegex.test(specifier) || forceRelative) && | ||
| !hasExtensionRegex.test(fileName) | ||
| ) { | ||
| if (relativePathRegex.test(specifier) && !hasExtensionRegex.test(fileName)) { |
There was a problem hiding this comment.
I can remove my stupid forceRelative hack now that I've made it so it returns relative paths instead of absolute.
| }/`; | ||
|
|
||
| specifier = pathSpecifier; | ||
| break; |
There was a problem hiding this comment.
Can't believe I missed this before, I'll be adding in proper caching soon for the resolve hook.
| if (TSConfig?.paths) { | ||
| for (const tsPath of Object.keys(TSConfig.paths)) { | ||
| const tsPathKey = tsPath.replace('/*', ''); | ||
| const tsPathKey = tsPath.replace('*', ''); |
There was a problem hiding this comment.
I feel like there could be a way to replace the * in tsPath and the path itself along with replacing the specifier, I tried a few ways to do it, but I can't find the flow/logic.
What I want it to do is somehow replace the specifier, tsPath and result in a single regex or statement.
| const tsResolvedPath = resolvePath( | ||
| resolvePath(baseURL, TSConfig.baseUrl!), | ||
| TSConfig.paths[tsPath][0].replace('*', specifier.split(tsPathKey)[1]), |
There was a problem hiding this comment.
There is likely a better way to do this, I can't find anything better myself.
|
@yoursunny If you don't mind, can you give this a quick look. Open to any feedback or suggestions, I'm just as uncertain about this method of handling the paths, but it does feel better, although there is room for improvement. |
|
https://github.com/microsoft/TypeScript/blob/84a3252e768b70dda70872b4ca095e66489bf1e0/src/compiler/moduleSpecifiers.ts#L120 |
## [1.5.1](v1.5.0...v1.5.1) (2020-05-27) ### Bug Fixes * **paths:** Refactor Paths handling ([#80](#80)) ([e537269](e537269))
|
🎉 This PR is included in version 1.5.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR refactors essentially the entire way the resolve function handles TSConfig paths to allow for single file "paths" along with allowing resolution of javascript files.