microsoft/node-jsonc-parser

difficulty bundling with esbuild

pedro-w opened this issue · 7 comments

pedro-w commented

I want to use node-jsonc-parser in a vscode extension and, per the guidance, I am using esbuild as the bundler. Unfortunately it doesn't work 'out of the box'; the internally required files (in src/impl/) are not bundled. I filed evanw/esbuild#1619 with esbuild. They explained the reason and suggested a work around, but also mentioned 'making the code (ie. node-jsonc-parser) more bundler friendly'.
I'd appreciate any advice on whether this is possible/straightforward to do, thanks in advance.

youngjuning commented

Have a same issues

lewisl9029 commented

Sounds like the solution could be as straightforward as building a CommonJS module instead of UMD for the main entry point script?

main is intended to be consumed by node, so it doesn't need any of the overhead UMD introduces over CJS. For browsers, there's already the module entry point which points to the ESM build.

If there's a known use case that requires UMD, perhaps it could be exposed using the browser field? If not, I'd vote for keeping things simple and only building ESM and CJS.

Happy to send a PR if this plan sounds good to @aeschli! 🙏

  • 👍8
aeschli commented

The module entry point points to the ESM code which is the best for bundlers such as webpack and 'esbuild'.

With webpack you need to add

resolve: {
    mainFields: ['module', 'main']
    ...
}

I'm not fluent with esbuild but think you might have to configure a similar thing.

The Monaco editor used UMD that's why we have it. We might no longer do so. Before removing the support, I'd like to double-check.

prisis commented

Hey @aeschli, would you accept a PR where https://github.com/developit/microbundle will be added to support umd,cjs and esm for this package?

felixhaeberle commented

I also fixed it with putting mainFields: ["module", "main"], into my context({}) of esbuild.

  • 👍3
  • 👎2
jakebailey commented

I accidentally pushed a crash out to a bunch of people last night due to this; it's totally my fault for not testing the bundle itself (testing is separate), but this is a very real problem. Hoping for #78.

  • 👍1