Broken build if a dependency's package.json uses the production export condition

[marbemac]

Environment

• Node v18.19.0
• Nitro current v2 branch

Reproduction

I've updated one of the nitro examples here to surface the bug: marbemac@9ecc35e

In this example I've added the @zedux/react package, which uses the production conditional in their package.json exports.

1. switch to that commit or copy the changes
2. pnpm i
3. cd examples/nano-jsx
4. pnpm build
5. node .output/server/index.mjs
6. open server url in browser, see error in console

Describe the bug

In the example described in the reproduction.

Here is what the original package.json looks like (from node_modules):

{
  "name": "@zedux/react",
  "exports": {
    ".": {
      "import": "./dist/esm/index.js",
      "require": "./dist/cjs/index.js",
      "production": {
        "import": "./dist/zedux-react.es.min.js",
        "require": "./dist/zedux-react.umd.min.js",
        "default": "./dist/zedux-react.umd.min.js"
      },
      "default": "./dist/cjs/index.js"
    }
  }
}

And here is what it looks like in the build .output folder's node_modules:

{
  "name": "@zedux/react",
  "exports": {
    ".": {
      "import": "./dist/zedux-react.es.min.js",
      "require": "./dist/zedux-react.umd.min.js",
      "production": {
        "import": "./dist/zedux-react.es.min.js",
        "require": "./dist/zedux-react.umd.min.js",
        "default": "./dist/zedux-react.umd.min.js"
      },
      "default": "./dist/zedux-react.umd.min.js"
    }
  }
}

The exports were correctly re-written to all match the production export (tests cover this here -> https://github.com/nitrojs/nitro/blob/v2/test/unit/externals.test.ts). However, note in the screenshot below that only the dist/esm folder is included in the build output's node_modules, perhaps because Nitro ignored the production conditional, and moved files over according to the original "import" export (before it is rewritten)? No idea as I'm not familiar with the Nitro internals, but maybe in the buildEnd step in the externals.ts rollup plugin.

This results in a mismatch, and runtime error when importing the affected package(s).

Additional context

I believe this will affect any projects with dependencies that make use of the production conditional in their package.json exports. Perhaps other conditionals, not sure. Might be related to #2242.

[marbemac]

Been investigating for the last 30 minutes and I the issue might actually be in https://github.com/vercel/nft. Seeing if I can reproduce with a test over there and will report back.

[pi0]

Thanks for follwup update. I will investigate better but quick look, production could be better the first condition in the lib. (consider ESM resolution order is based on the actual package exports keys order not conditions of resolution Node.js/NFT/Nitro, etc apply) so it should have been something like this for proper behavior:

{
  "name": "@zedux/react",
  "exports": {
    ".": {
      "production": {
        "import": "./dist/zedux-react.es.min.js",
        "require": "./dist/zedux-react.umd.min.js",
        "default": "./dist/zedux-react.umd.min.js"
      },
      "import": "./dist/esm/index.js",
      "require": "./dist/cjs/index.js",
      "default": "./dist/cjs/index.js"
    }
  }
}

[pi0]

And a quick workaround is to add to traceInclude in nitro config to trace all files:

externals: { traceInclude: "@zedux/react" }

(you can double check but I think also if in production you apply NODE_ENV=production, Node.js resolver also should resolve /dist/zedux-react.es.min.js which is first I guess still have to double check)

[marbemac]

Good tip on the traceInclude! I'll try it as a stop gap.

Does indeed look like NFT is picking whichever export key matches first. That combined with how NFT implicitly includes "default", "require", and "import" conditions means that they will always be picked if defined earlier in the exports object. https://github.com/vercel/nft/blob/main/src/resolve-dependency.ts#L166-L172

Is that how the node resolver works? Might be - and if so perhaps the issue is the implicit conditions that are always added by NFT?

[pi0]

Is that how the node resolver works?

Yes, exports keys order matters for ESM resolution algorithm in general (this is also why typescript has to always go first for example)

[marbemac]

Yeah just verified, ok well then looks like it's an issue in the downstream dependency, as you suggested, thanks for your help!

[marbemac]

@pi0 thinking on it a bit more, would it make sense for nitro to adhere to the same node resolver behavior when re-writing the exports package.json field for external deps? Right now it seems to always attempt to use production, regardless of position in the exports: https://github.com/nitrojs/nitro/blob/v2/src/rollup/plugins/externals.ts#L485

Then again, not a huge deal I suppose, and might need to be considered a breaking change to change this behavior.

[pi0]

Yes you are right, nitro could rewrite to first matched condition instead of production hardcoded. Might be little tricky but doable.