fix(ext/node): fix unable to resolve fraction.js #18544
Conversation
ext/node/ops.rs
Outdated
| let orignal = modules_path.clone(); | ||
| let mod_dir = path_resolve(vec![modules_path, name]); | ||
| if Path::new(&mod_dir).is_dir() { | ||
| mod_dir | ||
| } else { | ||
| orignal | ||
| } |
There was a problem hiding this comment.
This is the fix here. I'm not too happy about it because it adds a file system check, but it does solve the issue.
The reason I'm not happy about it is that the resolution code expects node_module paths everywhere like /my-project/node_modules. But in this branch here we ignore that assumption and push a module directory to the list of paths instead (/my-project/node_modules/my-module). Then we reach this section here where we'd incorrectly append the package name to the path again resulting in /my-project/node_modules/my-module/my-module which doesn't exist and therefore panics.
There was a problem hiding this comment.
Good catch. IIRC this was done to support CJS in situation where we use "global cache" (ie. not creating local node_modules directory), but appears that it's wrong. I'm happy to completely reorganize this bit of code so we don't have to do is_dir() check.
marvinhagemeister
force-pushed
the
resolve-autoprefixer
branch
from
8974653
to
aad0269
Compare
marvinhagemeister
force-pushed
the
resolve-autoprefixer
branch
from
aad0269
to
6627d4f
Compare
Added more methods to `ext/node/clippy.toml` that are not allowed to be used in the crate. Prerequisite for #18544
|
@bartlomieju Thanks for bringing this PR over the finish line 💯 |
Added more methods to `ext/node/clippy.toml` that are not allowed to be used in the crate. Prerequisite for #18544
Turns out
autoprefixeris a better reproduction case thenmicrobundle.Fixes #18535
Fixes #18600