Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bundling doesn't pull in all required dependencies #1619

Closed
Tracked by #4042
pedro-w opened this issue Sep 20, 2021 · 5 comments
Closed
Tracked by #4042

bundling doesn't pull in all required dependencies #1619

pedro-w opened this issue Sep 20, 2021 · 5 comments

Comments

@pedro-w
Copy link

pedro-w commented Sep 20, 2021

I want to use esbuild on an npm project using jsonc-parser. esbuild seems to run OK but the bundle it creates does not contain all the required code.

PS D:\temp\z\esbuild-example> npm run build

> esbuild-example@1.0.0 build D:\temp\z\esbuild-example
> npm run esbuild-base -- --sourcemap


> esbuild-example@1.0.0 esbuild-base D:\temp\z\esbuild-example
> esbuild ./index.js --bundle --outfile=dist/bundle.js --platform=node "--sourcemap"


  dist\bundle.js      4.7kb
  dist\bundle.js.map  8.9kb

⚡ Done in 14ms
PS D:\temp\z\esbuild-example> node .\dist\bundle.js
internal/modules/cjs/loader.js:892
  throw err;
  ^

Error: Cannot find module './impl/format'
Require stack:
- D:\temp\z\esbuild-example\dist\bundle.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:889:15)
    at Function.Module._load (internal/modules/cjs/loader.js:745:27)
    at Module.require (internal/modules/cjs/loader.js:961:19)
    at require (internal/modules/cjs/helpers.js:92:18)
    at D:\temp\z\esbuild-example\dist\bundle.js:41:23
    at D:\temp\z\esbuild-example\dist\bundle.js:31:17
    at node_modules/jsonc-parser/lib/umd/main.js (D:\temp\z\esbuild-example\dist\bundle.js:37:7)
    at __require2 (D:\temp\z\esbuild-example\dist\bundle.js:12:44)
    at Object.<anonymous> (D:\temp\z\esbuild-example\dist\bundle.js:113:38)
    at Module._compile (internal/modules/cjs/loader.js:1072:14) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'D:\\temp\\z\\esbuild-example\\dist\\bundle.js' ]
}

I was able to produce a bundle using webpack.

Platform: Windows 10
Version: esbuild 0.12.28
Node: v14.17.5

@pedro-w
Copy link
Author

pedro-w commented Sep 20, 2021

I have put a minimal example in https://github.com/pedro-w/esbuild-example

@hyrious
Copy link

hyrious commented Sep 20, 2021

This is because the jsonc-parser library exports a UMD file which uses require() indirectly, which prevents esbuild to analyze and bundle these dependencies.

As a quick solution, you could use its ESM target in the module field by appending this option:
--main-fields=module,main.

// CORRECT, esbuild could know it depending on 'react'
var react = require('react')

// INCORRECT, esbuild can not know the dependencies
var r = require, react = r('react')

@pedro-w
Copy link
Author

pedro-w commented Sep 21, 2021

Thanks for the swift response. I can confirm that this does fix the problem for me, though to be honest I don't fully understand it! Looking at the docs it seems that the default for node would be the equivalent of --main-fields=main,module, is that right? I do see what you mean about indirect requires, in the node_modules/jsonc-parser/lib/umd/main.js, require is passed in as a parameter to a function and called from there.
If there were a 'not-quick' solution, what would it be? Would I have to get MS to change something in jsonc-parser?

@hyrious
Copy link

hyrious commented Sep 21, 2021

Looking at the docs it seems that the default for node would be the equivalent of --main-fields=main,module, is that right?

Right. This is mostly because people often incorrectly use the module field for browser-specific code. It is correct for esbuild to prefer main over module when targetting node.js, to prevent potential bundling code that only runs on browser.

Would I have to get MS to change something in jsonc-parser?

MS do have to change the package, by making its main file in CJS format which directly uses require, or by adding an exports field which helps you select the bundled file. There're many choices to do the right thing, a common practice would be using the node.js's built-in es modules (since node 13).

This story would be too long to put here. If you'd like to learn "How to make a correct NPM package which runs on {Node.js {CJS | ESM} | Browser {script | script type=module}} context", You'd better look at some modern packages like Sindre Sorhus' pure ESM packages, vue@next, ...

I'd like to share my pkg marshal, which works perfectly in Node.js CJS/ESM context and in bundlers (always choose the ESM one) and in browser (having an IIFE file). Again, this library is pure enough to do so. When the code has some side-effects (like a global state) that must be only run once, things will get more complex.

If there were a 'not-quick' solution, what would it be?

Did I say esbuild prefer main over module when targetting node.js just for compatibility? If you know --main-fields=module,main won't break on node.js, then just use it.

For bigger projects that depend on many packages, you may want to adjust the file to be imported, which could be made by writing a plugin that helps esbuild finding the correct file in the onResolve callbacks.

@evanw
Copy link
Owner

evanw commented Sep 22, 2021

Having esbuild handle this automatically is out of scope since esbuild only handles require calls that can be statically-evaluated at compile time without partially-evaluating the input JavaScript code. The package not being bundler-friendly is a problem with the package, not with esbuild.

As mentioned above, workarounds are:

  • Use --main-fields=module,main (may cause issues with other packages)
  • Write an esbuild plugin using onResolve to swap out just this one file (won't affect other packages)
  • Get Microsoft to make their code bundler-friendly

Closing as out of scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants