microsoft / node-jsonc-parser

Scanner and parser for JSON with comments.
MIT License
599 stars 53 forks source link

difficulty bundling with esbuild #57

Open pedro-w opened 3 years ago

pedro-w commented 3 years ago

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 https://github.com/evanw/esbuild/issues/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 3 years ago

Have a same issues

lewisl9029 commented 2 years ago

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! 🙏

aeschli commented 2 years ago

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.

tjx666 commented 1 year ago

for anybody find solution ref: https://github.com/tjx666/awesome-vscode-extension-boilerplate/blob/main/scripts/esbuild.mts#L28, I find solution from https://github.com/vuejs/language-tools/blob/master/packages/vscode-vue/scripts/build.js

prisis commented 1 year ago

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 1 year ago

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

jakebailey commented 8 months ago

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.