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

difficulty bundling with esbuild #57

Open
pedro-w opened this issue Sep 24, 2021 · 7 comments · May be fixed by #78
Open

difficulty bundling with esbuild #57

pedro-w opened this issue Sep 24, 2021 · 7 comments · May be fixed by #78

Comments

@pedro-w
Copy link

pedro-w commented Sep 24, 2021

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
Copy link

Have a same issues

@lewisl9029
Copy link

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
Copy link
Contributor

aeschli commented Feb 7, 2022

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.

devversion added a commit to devversion/dev-infra that referenced this issue Oct 21, 2022
We recently prioritized `module` over `main` but this results in browser
code being prioritized sometimes. We revert that change and keep the
original/default ESBuild main field order.

This unveils an issue with `jsonc-parser` though- which ships an UMD
file for `main` that cannot be bundled. We can work around this by
processing `jsonc-parser`'s ESM output directly and then using that
instead of leaving resolution to ESBuild for this package.

There is an issue for this being tracked: microsoft/node-jsonc-parser#57
devversion added a commit to devversion/dev-infra that referenced this issue Oct 21, 2022
We recently prioritized `module` over `main` but this results in browser
code being prioritized sometimes. We revert that change and keep the
original/default ESBuild main field order.

This unveils an issue with `jsonc-parser` though- which ships an UMD
file for `main` that cannot be bundled. We can work around this by
processing `jsonc-parser`'s ESM output directly and then using that
instead of leaving resolution to ESBuild for this package.

There is an issue for this being tracked: microsoft/node-jsonc-parser#57
devversion added a commit to devversion/dev-infra that referenced this issue Oct 21, 2022
We recently prioritized `module` over `main` but this results in browser
code being prioritized sometimes. We revert that change and keep the
original/default ESBuild main field order.

This unveils an issue with `jsonc-parser` though- which ships an UMD
file for `main` that cannot be bundled. We can work around this by
processing `jsonc-parser`'s ESM output directly and then using that
instead of leaving resolution to ESBuild for this package.

There is an issue for this being tracked: microsoft/node-jsonc-parser#57
devversion added a commit to angular/dev-infra that referenced this issue Oct 21, 2022
We recently prioritized `module` over `main` but this results in browser
code being prioritized sometimes. We revert that change and keep the
original/default ESBuild main field order.

This unveils an issue with `jsonc-parser` though- which ships an UMD
file for `main` that cannot be bundled. We can work around this by
processing `jsonc-parser`'s ESM output directly and then using that
instead of leaving resolution to ESBuild for this package.

There is an issue for this being tracked: microsoft/node-jsonc-parser#57
@prisis
Copy link

prisis commented Jun 6, 2023

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
Copy link

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

@remcohaszing remcohaszing linked a pull request Aug 21, 2023 that will close this issue
@jakebailey
Copy link
Member

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.

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

Successfully merging a pull request may close this issue.

8 participants