kentcdodds/mdx-bundler

Incorrect resolution for nested `node_modules` due to usage of `basedir` in NodeResolvePlugin

amoshydra opened this issue · 1 comments

  • mdx-bundler version: 10.0.3
  • node version: 18.8.0, 18.14.1, 20.12.1
  • npm version: 8.18.0, 9.5.0, 10.5.0
  • esbuild 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.

mdx-bundler/src/index.js

Lines 147 to 150 in 616c5b0

NodeResolvePlugin({
extensions: ['.js', '.ts', '.jsx', '.tsx'],
resolveOptions: {basedir: cwd},
}),

https://github.com/kentcdodds/mdx-bundler/pull/68/files#diff-bfe9874d239014961b1ae4e89875a6155667db834a410aaaa2ebe3cf89820556R149

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 imports package-b imports package-a
    • package-a should resolve to node_modules/package-a/a.js
  • index-c imports package-c imports package-a
    • package-a should resolve to node_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 imports package-b imports package-a
    • package-a resolved to node_modules/package-a/a.js
  • index-c imports package-c imports package-a
    • package-a resolved to node_modules/package-a/a.js <-- ❌ incorrect resolution

Reference:

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 plugin
  • esbuild with @esbuild-plugins/node-resolve using default option
  • esbuild with @esbuild-plugins/node-resolve passing project's working directory as basedir

Observation

dist folder contain the output for each build

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 load md file if plugin is removed. mdx loads fine)
  • passing our own @esbuild-plugins/node-resolve without the basedir option
    const 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