-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
ESM modules and json importing [feature request] #20494
Comments
The current ESM implementation already allows to import a JSON as default (first line). /cc @nodejs/modules |
named exports from json doesn't make a whole lot of sense to me. named imports are not and shouldn't be used in place of destructuring. |
I have some concerns with JSON imports that I can list here:
|
a json file should only have a default export; named imports are decidedly NOT destructuring, and allowing this would spread that confusion. |
Named exports work for the cases they do and don't for the cases they don't. Users don't care about the cases named exports don't work since they can use alternative import forms to grab them. It's a convenience thing. JSON is just another |
@jdalton I don't understand the major wins vs just using destructuring which is also static so tree shaking isn't a problem. The edge cases are weird. Adding weirdness for the sake of convenience I think needs to have a more compelling reason than what exists in this thread currently to me personally. |
JSON support isn't something that has to be explicitly expanded or deliberated on. It shakes out naturally, with no extra work, with wider CJS named exports support as seen with Babel and |
That it’s no extra work doesn’t mean it’s a good idea. Even if named exports from CJS are able to become a thing, i don’t think that should be made to apply to json imports. |
Named exports for CJS is an interop enhancement. As such it doesn't seem like the place to start green-fielding and diminishing value. |
@jdalton I don't see it as no extra work / naturally shaking out from how JSON works? I'm not sure I understand how this affects my concerns above. |
In CJS Node supports loading CJS named exports work in the cases they can work. It's an interop enhancement which means, yes, some cases like property names that can't be translated into identifiers won't be supported as a named export and that's fine. That isn't something supported by Babel or others anyways. |
@jdalton we are not talking about CJS named exports here. I disagree on your course of discussion unless we tie these features together. |
with cjs, everything is an object with esm, everything is well-known values json is, by definition, a representation of an object. saying that "because we do it in cjs we should do it in esm" is in my opinion a very silly stance because they are inherently different. |
You may not have thought you were talking about CJS named exports, but that's what it is and more importantly that's how the user will see it. JSON is just another
A
When the feature discussed is CJS interop then it totally makes sense to bring up. |
I think it’s reasonable to bring up. I think it’s a choice we could certainly make. I agree that the implementation would shake out “for free” along with named imports for CJS, and that non-identifier-names would simply not be named-importable, and that would be fine. However, conceptually, i do not think json is the same as a CJS module, and i think that it would be a mistake to let the consumer of a json file “conveniently” but sloppily pretend that their json file is exporting more things than “the single json-parsed value”. |
The arm wrestle between green-fielding and interop is ongoing for sure. None that will get settled in this thread. |
That is debatable but I side on JSON not being CJS. If we introduce named exports we might also need to move it off of the CJS It seems like this feature could be added after shipping an implementation of ESM, and I'm still not sure the convenience is worth prioritizing. We can add this to the modules meeting agenda, but probably cannot do anything while we still debate use cases. |
It could also be bundled with however wider CJS named exports ends up being handled.
I don't think there is any urgency around this to warrant a specific agenda item.
I think it'll likely work out of the use case / feature process on its own. |
Just to contribute a use case, one pattern I see often is |
@GeoffreyBooth I don't think anyone wants to prevent loading JSON here. |
@GeoffreyBooth yeah, that's the typical scenario: getting |
Webpack supports named exports for JSON: import { version } from './package.json';
console.log(version); Presumably, it does this to simplify tree shaking; the unused portions of the JSON can be dropped. I could see it being useful if Node mimicked this behaviour, mostly for code that is designed to run on both the server and the client. It would also serve to make porting to natively supported .mjs easier; devs would not have to refactor such instances in their code just to get it to run on Node. |
Code that runs in browsers cannot import JSON, only JS modules. |
i'm not convinced this should be in core. this seems like a good situation for some sort of |
It's worth noting we still don't even have any consensus on the minimal implementation that ".json" imports should be supported (to rather possibly be handled by asset loading techniques). |
I was referring to code compiled by webpack for browsers (which I expect will still be a thing for a long time to come), but there's also WICG/webcomponents#770 for native support. |
Hasn't this issue been addressed by #26745? |
Yes, thanks @aduh95 |
I landed at this issue while looking for an easy way to achieve what has been originally requested: import pkg from './package'
import { name, version } from './package' After trying a few approaches, I came up with following, without dependencies or feature flags: import { readFile } from 'fs/promises'
const pkg = JSON.parse(await readFile(new URL('./package', import.meta.url)))
const { name, version } = pkg; |
@kalinchernev you may find the following a bit better. import { createRequire } from 'module';
const require = createRequire(import.meta.url);
const { name, version } = require('./package'); |
Thanks @MylesBorins! |
I was quite surprised that it's not possible to import |
It's not a bug. It's an experimental feature currently behind a flag ( |
Will it be possible in the future to import .json files as we used to required .json files?
The text was updated successfully, but these errors were encountered: