Should not use dependency's package.json "module" field when targeting node env

[3cp]

🐛 bug report

When bundling a nodejs lib with includeNodeModules, parcel wrongly brought in dependency's "module" file instead of "main" file. This caused the bundle file failed to run.

🎛 Configuration (.babelrc, package.json, cli command)

Demo project https://github.com/3cp/parcel-lib-bug More details in the readme of the demo.

🤔 Expected Behavior

The "module" field (and also "browser" field) in package.json is NOT npm standard. In Nodejs environment, it's never utilised.

The "module" and "browser" fields are traditions developed by various web app bundler, starting with browserify I believe.

It's understandable for parcel to utilise dependency's "module" field when bundling for browser target. However when targeting Nodejs lib, parcel should follow Nodejs practice and avoid peeking "module" and "browser" fields.

😯 Current Behavior

When targeting Nodejs env, parcel still uses "module" filed in the dependency. This behaviour does not match the real Nodejs environment.

💁 Possible Solution

Separate behaviour of Nodjs target, it should strictly follow Nodejs module resolution. For any dependency package, only use "main", "type", "exports" fields in the package's package.json, do not use "module" and "browser" fields which are designed for browser target only.

🔦 Context

The issue prevents me from migrating from ncc to parcel to bundle https://github.com/makesjs/makes since it uses "mri" package.

💻 Code Sample

Shown above.

🌍 Your Environment

Software	Version(s)
Parcel	2.4.1.
Node	14.18.1.
npm/Yarn	npm v6.14.15
Operating System	macOS 12.3.1

[devongovett]

The browser field will not be used when targeting node. The module field doesn't imply a target however and will be used in order to improve tree shaking.

I think the problem here is that the mri package is broken with scope hoisting. Either due to that package being broken, or due to a bug in Parcel. I haven't investigated yet. I don't think we should change the resolution behavior though.

[3cp]

mri does support commonjs and esm in different export. When using "module" in a webapp, it's using default exports like import mri from "mri".

When using in Nodejs env's main file, it's using module.exports, like const mri = require("mri"). Since mri's npm package is in commonjs format (the "main" file, and the missing "type" field), this is the designed use case in Nodejs env.

The problem with parcel, is that it should follow Nodejs module resolution in Nodejs target env. If a source code is runnable in Nodejs, the bundled dist file from parcel should behave exactly same.

In Nodejs running env, package.json "module" is never in play, it's not part of npm standard.

[devongovett]

The module field may not be used by node.js itself, but it is used by all bundlers to get an ES module version of a library. I don't think we can change that now.

The actual issue here is that scope hoisting is broken for this module. The resolution behavior is correct and matches other bundlers.

[3cp]

by all bundlers to get an ES module version of a library

https://github.com/vercel/ncc (webpack based) does not do that. I am using ncc right now, and try to move to parcel.

microbundler (rollup based) does not support project source code in commonjs, so I cannot use it for my nodejs project. I don't know how it behaves.

The mri package problem is that the commonjs version dist/index.js doesn't match dist/index.mjs exports at all. I agree it's a strange practice.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs.