Incorrect resolution for nested `node_modules` due to usage of `basedir` in NodeResolvePlugin
amoshydra opened this issue · 1 comments
mdx-bundler
version: 10.0.3node
version: 18.8.0, 18.14.1, 20.12.1npm
version: 8.18.0, 9.5.0, 10.5.0esbuild
version: 0.19.12, 0.23.1@esbuild-plugins/node-resolve
version: 0.2.2
Relevant code or config
const result = await bundleMdx({
file: "c:/project/index-c.mdx",
cwd: "c:/project",
});
// c:/project/index-c.mdx
import packageC from "package-c";
<span>{packageC}</span>
I have identified that usage of basedir
in NodeResolvePlugin causes the incorrect module resolution to occur for nested node_modules
.
Lines 147 to 150 in 616c5b0
What you did:
Attempt to bundle a component using package (package-a) with 2 different versions, imagine the node_modules
structure like this
project
├── index-b.js
├── index-c.js
└── node_modules
├── package-a (1.0.0)
│ ├── a.js
│ └── package.json
├── package-b
│ ├── b.js
│ └── package.json
└── package-c
├── c.js
├── node_modules
│ └── package-a (2.0.0)
│ ├── a.js
│ └── package.json
└── package.json
* exact version doesn't matter here, only the folder structure matter
index-b
importspackage-b
importspackage-a
package-a
should resolve tonode_modules/package-a/a.js
index-c
importspackage-c
importspackage-a
package-a
should resolve tonode_modules/package-c/node_modules/package-a/a.js
What happened:
The incorrect module is resolved.
mdx-bundler
always resolve module at ${process.cwd()}/node_modules
, breaking NodeJS's / ESBuild node module resolution logic.
index-b
importspackage-b
importspackage-a
package-a
resolved tonode_modules/package-a/a.js
index-c
importspackage-c
importspackage-a
package-a
resolved tonode_modules/package-a/a.js
<-- ❌ incorrect resolution
Reference:
esbuild
- implements NodeJS module resolution... implemented esbuild's path resolution by following node's own documentation ...
- evanw/esbuild#1117 (comment)- NodeJS module resolution
- https://nodejs.org/docs/latest-v20.x/api/modules.html#loading-from-node_modules-folders
@esbuild-plugins/node-resolve
- by default follow NodeJS module resolution ifbasedir
is not passed, and start looking fornode_modules
from the caller's modulevar basedir = opts.basedir || path.dirname(caller());
basedir
is default to the directory of thecaller
module when not provided- https://github.com/browserify/resolve/blob/v1.22.8/lib/async.js#L107
Reproduction repository:
A basic reproduce repo is created to demonstrate this point using only esbuild
and @esbuild-plugins/node-resolve
https://github.com/amoshydra/repro-kentcdodds-mdx-bundler/tree/repro-issue-233
Test setup
build.mjs contains 3 setup:
esbuild
without pluginesbuild
with@esbuild-plugins/node-resolve
using default optionesbuild
with@esbuild-plugins/node-resolve
passing project's working directory as basedir
Observation
dist
folder contain the output for each build
- /dist/default/index-c.js
- /dist/resolve-default/index-c.js
- /dist/resolve-project-directory-as-basedir/index-c.js
Problem description:
mdx-bundler
should follow NodeJS module resolution, but it does not.
Consequently, as we migrate our bundler from webpack/docz
to mdx-bundler
, we observe many of the packages failed to run due to incorrect module resolution
Suggested solution:
For the scope of my project, the following workarounds works for me:
removing@esbuild-plugins/node-resolve
, let esbuild perform the resolution
(mdx-bundler cannot loadmd
file if plugin is removed.mdx
loads fine)- passing our own
@esbuild-plugins/node-resolve
without thebasedir
optionconst result = await bundleMdx({ file: "c:/project/document/document.mdx", cwd: "c:/project", esbuildOptions: (options) => { const nodeResolvePluginIndex = options.plugins.findIndex(({ name }) => name === "node-resolve"); if (nodeResolvePluginIndex >= 0) { // replace original plugin with our own without `resolveOptions.basedir` configured options.plugins.splice(nodeResolvePluginIndex, 1, NodeResolvePlugin({ extensions: [".ts", ".tsx", ".js", ".jsx"], }); } return options; }, });
Earlier, I wanted to suggest removing basedir
.
However basedir
appear to be added intentionally in #68. I have yet to look into what problem #68 is trying to solve
I am unable to reproduce the issue raised in #66 / #68 for these combination of versions:
package | version | depended |
---|---|---|
node |
18.8.0 , 18.14.1 , 20.12.1 |
|
npm |
8.18.0 , 9.5.0 , 10.5.0 |
|
mdx-bundler |
10.0.3 |
|
esbuild |
0.19.12 , 0.23.1 |
by mdx-bundler |
@esbuild-plugins/node-resolve |
0.2.2 |
by mdx-bundler |
resolve |
1.22.8 |
by @esbuild-plugins/node-resolve |
I have thus raised #234