fix(tsconfig): respect path mapping#136
fix(tsconfig): respect path mapping#136jjangga0214 wants to merge 1 commit intodependents:masterfrom
Conversation
mrjoelkemp
left a comment
mrjoelkemp
left a comment
There was a problem hiding this comment.
Thanks for contributing. I think this logic best belongs in filing-cabinet: https://github.com/dependents/node-filing-cabinet/blob/754d3889e8ba53283a126eb69f08c71b760c8c2b/index.js#L214. That's the module that deals with resolving paths based on module types. Since this logic is specific to typescript path resolution, it fits well there.
It would be awesome to provide a test with this code once it's ported to filing-cabinet. There must be something in the lerna repo that could be added to the filing-cabinet suite so that we can refactor freely without worrying about breaking the resolution logic you'll add. See other TS path resolution tests here: https://github.com/dependents/node-filing-cabinet/blob/master/test/test.js#L391.
Thanks again for the effort here, but heavy TS logic doesn't belong in this module.
| return createMatchPath(absoluteBaseUrl, config.tsConfig.compilerOptions.paths) | ||
| } | ||
| return (alias) => undefined; | ||
| })() |
There was a problem hiding this comment.
Is there a reason you create intermediate IIFEs for this and https://github.com/dependents/node-dependency-tree/pull/136/files#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R130? It doesn't seem necessary.
There was a problem hiding this comment.
@mrjoelkemp That is to prevent using let over const. (A matter of personal preference, so I understand even if you don't like it.)
For example, let's say we should initialize foo by some condition.
// In this example, assume a ternary operator cannot be used,
// as if-else blocks can be large in real cases.
let foo
if (bar) {
foo = 1
} else {
foo = 2
}foo is declared as let, so it can be mutated from somewhere else when it should not be.
To prevent this, some (especially functional) languages provides syntax sugar or special approach.
For instance, Rust can treat if-else as an "expression", not a "statement".
// In Rust, pure `let` without `mut` keyword is equivalent to `const` in Javascript.
let foo = if bar {
1
} else {
2
};In Javascript, calling IIFE for variable initialization is a similar pattern.
// foo now becomes `const` !
const foo = (() => {
if (bar) {
return 1
}
return 2
})()
|
Thanks! I created new PRs: |
2 participants
Hi!
First, thanks for this awesome package!
I tested this PR against https://github.com/jjangga0214/ts-yarn-lerna-boilerplate, and got success.
I used tsconfig-path for path mapping.
Thanks.
Close #135