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

Support named imports for JSON #2

Closed
antfu opened this issue May 12, 2022 · 3 comments · Fixed by esbuild-kit/esm-loader#3 or esbuild-kit/esm-loader#2
Closed

Support named imports for JSON #2

antfu opened this issue May 12, 2022 · 3 comments · Fixed by esbuild-kit/esm-loader#3 or esbuild-kit/esm-loader#2
Labels
bug Something isn't working outdated

Comments

@antfu
Copy link
Sponsor Contributor

antfu commented May 12, 2022

https://stackblitz.com/edit/node-ewwreq

@antfu antfu changed the title Does not support named imports for JSON Support named imports for JSON May 13, 2022
@privatenumber
Copy link
Owner

This problem is happening because Node.js is unable to detect whether a .ts file is commonjs/module.

I worked on a fix in esbuild-kit/esm-loader#2, which detects the package type (by detecting nearest package.json), and skips it if it's commonjs, so that it can be handled by cjs-loader.

When the fix is merged, named imports on JSON will work for CommonJS contexts.

However, it still won't work in Module contexts because native JSON modules don't support named imports by spec. We can technically override this behavior to support it but I'm not sure if we should because we don't back-port things like __filename/__dirname.

What do you think?

@privatenumber privatenumber added the bug Something isn't working label May 13, 2022
@antfu
Copy link
Sponsor Contributor Author

antfu commented May 13, 2022

I think using named import json is quite a common convention supported by most of the bundlers. Importing a single field of a json would also enable tree-shaking for a smaller footprint. I would expect this as I might run the .ts file with the runtime in development and use bundler for production. Without this, it would be quite a hassle to deal with. I am not sure if sacrificing this to match the spec is worth it here.

@privatenumber
Copy link
Owner

Fair point. I actually encountered that exact problem when importing version from package.json, which works in Rollup/Vite.

I'm OK with overriding JSON modules to add support in esm-loader as well.

For reference on bundlers:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working outdated
Projects
None yet
2 participants