Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Feature: Import JSON without needing asynchronous syntax #114

Closed
GeoffreyBooth opened this issue May 20, 2018 · 31 comments
Closed

Feature: Import JSON without needing asynchronous syntax #114

GeoffreyBooth opened this issue May 20, 2018 · 31 comments
Labels

Comments

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented May 20, 2018

Updated feature request description:

Imagine code like this in current Node:

const packageJson = JSON.parse(fs.readFileSync('./package.json'));
console.log(`Running version ${packageJson.version}`);

This can also be written as:

const packageJson = require('./package.json');
console.log(`Running version ${packageJson.version}`);

Both of these examples are written without asynchronous syntax on the user’s part: there are no callbacks, no promises, no await. Perhaps Node is doing asynchronous stuff behind the scenes, but the user is unaware of it. require appears to be as synchronous as readFileSync.

This feature request is that a user’s code to import JSON files via ESM should be like these examples: no callbacks, no promises, no await. This feature request doesn’t concern Node’s implementation under the hood, and whether it is synchronous or asynchronous; the request is only that the user’s code be able to be written in a synchronous style.

Use case 48. Part of #100.


Original feature request description:

ESM imports of JSON can be written in a synchronous style:

import { version } from './package.json';
console.log(`Running version ${version}`);

The point of this feature is that users can use the same synchronous style of coding (in the vein of fs.readFileSync as opposed to callback- or promise-based alternatives) when the import is of JSON. Or put another way, users aren’t required to use await or promises or other asynchronous constructs in code in order to import JSON.

Use case 48. Part of #100.

@devsnek
Copy link
Member

devsnek commented May 20, 2018

i don't understand this at all. assuming the "synchronous" part is that its being statically imported, it would be better to then just say that, as there's nothing inherently (a)synchronous about static imports.

if this isn't referring to the static import then it will definitely need to be clarified further.

@GeoffreyBooth
Copy link
Member Author

It’s just that the import doesn’t return a promise, regardless of the module type of what is to be imported. I can import something and then immediately use it in the next line:

import { keys } from 'underscore'; // CommonJS package
const dateKeys = keys(Date);

@devsnek
Copy link
Member

devsnek commented May 20, 2018

so like i said above, maybe this should be rephrased?

@GeoffreyBooth
Copy link
Member Author

What would you have it say? I’m not sure I understand.

The only reason this feature is here is because there was some discussion of handling CommonJS imports via async import(). This feature is a request that the regular ES2015 import statement be usable for CommonJS.

@devsnek
Copy link
Member

devsnek commented May 20, 2018

anything that can be statically imported can be dynamically imported and vice-versa. there is no difference in how they are handled.

neither the use of dynamic or static import informs whether the user agent is actually processing import requests synchronously or asynchronously.

@devsnek
Copy link
Member

devsnek commented May 20, 2018

poorly phrased dupe of #100

@devsnek devsnek closed this as completed May 20, 2018
@GeoffreyBooth
Copy link
Member Author

Let's not close features without consensus, if you don't mind? As I wrote at the top, this is part of #100. That one is so big that I thought it would be best to discuss its parts in separate threads.

@GeoffreyBooth GeoffreyBooth reopened this May 20, 2018
@devsnek
Copy link
Member

devsnek commented May 20, 2018

this issue is literally meaningless. all it says once you degarble it is "allow us to import cjs" which already is what #100 is. specifically stating that you want static imports to work is pointless, and saying "synchronous" is at best a misunderstanding and at worst intentionally misleading.

@benjamingr
Copy link
Member

@devsnek I read this issue as automatically creating named exports from JSON files.

  • We can do this without supporting named imports for cjs
  • JSON is statically analyzable unlike regular CJS, so we can support this even without late bindings.

@benjamingr
Copy link
Member

That said, I'm having a very hard time following all these issues.

@GeoffreyBooth
Copy link
Member Author

this issue is literally meaningless . . . at best a misunderstanding and at worst intentionally misleading.

I would appreciate a more collaborative tone here.

This was an item of its own on the features document. It also seemed to me like a subset of #100, which is also its own item on the features document, but because that one is so huge I’m proposing we split that one up into smaller chunks and then close #100. In the interest of not making unilateral decisions, though, I wanted to leave it to the group to find consensus on how these should be organized. Perhaps that should be a topic for the next meeting.

@benjamingr You are correct, but don’t forget that the feature included CommonJS, not just JSON. I think it’s as simple as it seems, that with regards to importing non-ESM modules, users are allowed to code in a synchronous style (in the vein of fs.readFileSync as opposed to fs.readFile with a callback, or a promise-based version). That’s really it. It’s a cousin of #87, of saying that users can continue using the syntax they’re used to regardless of whether the member to be imported is ESM or CommonJS. I’ve edited the original post to add more detail, please let me know if it’s still unclear.

@benjamingr
Copy link
Member

benjamingr commented May 20, 2018

@benjamingr You are correct, but don’t forget that the feature included CommonJS, not just JSON

Then is it possible to open a separate issue for JSON?

@GeoffreyBooth GeoffreyBooth changed the title Feature: Synchronous ESM loading Feature: Import JSON without needing asynchronous syntax May 20, 2018
@GeoffreyBooth
Copy link
Member Author

@benjamingr Sure, I think I’ll keep this one as JSON (since you’re already discussing JSON above) and I opened #116 for the CommonJS variant.

@devsnek
Copy link
Member

devsnek commented May 20, 2018

@GeoffreyBooth

I would appreciate a more collaborative tone here.

i'm not trying to say the thing this issue describes is a bad idea. i'm saying the way this issue describes it is confusing and at this point i'm going to consider it intentionally inaccurate because i find it hard to believe you haven't read my messages.

so...

please please please,

please please,

please:

stop using the terms "synchronous" and "asynchronous" to describe the syntax of dynamic and static imports.

instead, use the terms "static" and "dynamic"

my quote from above:

anything that can be statically imported can be dynamically imported and vice-versa. there is no difference in how they are handled.

neither the use of dynamic or static import syntax informs whether the user agent is actually processing import requests synchronously or asynchronously.

once again:

anything that can be statically imported can be dynamically imported and vice-versa. there is no difference in how they are handled.

neither the use of dynamic or static import syntax informs whether the user agent is actually processing import requests synchronously or asynchronously.

@ljharb
Copy link
Member

ljharb commented May 20, 2018

That’s not entirely accurate; the spec requires import() to be processed asynchronously (if the module has not yet been evaluated).

@devsnek
Copy link
Member

devsnek commented May 20, 2018

@ljharb thanks for the clarification, but the thing i'm more worried about is "import 'x' is synchronous syntax" which i think is quite a misleading thing to claim.

@benjamingr
Copy link
Member

The issue is really "export JSON root properties as names when importing a JSON file".

It has nothing to do with synchronous/asynchrous nor static/dynamic.

@benjamingr
Copy link
Member

Also I recommend this section in @2ality's book in case anyone still has doubts about the difference between named exports in ESM and CJS.

@GeoffreyBooth
Copy link
Member Author

This is what I consider to be synchronous syntax:

const packageJson = fs.readFileSync('./package.json', 'utf-8');
console.log(`Running version ${packageJson.version}`);

This is what I consider to be asynchronous syntax:

fs.readFile('./package.json', 'utf-8', function(err, packageJson) {
  console.log(`Running version ${packageJson.version}`);
}
// Or the promise-based equivalent

It’s right there in the name: Sync. I’m referring to the code a user writes, not to anything that might happen under the hood.

I’m feeling bullied in this thread. I don’t need to be ordered what terms to use. I also don’t need to accused of bad faith or intentionally misleading people. I don’t find such comments to be likely to lead to a productive collaboration.

@devsnek
Copy link
Member

devsnek commented May 20, 2018

@GeoffreyBooth both of those examples can be "plugged in" to static import syntax without the user noticing. right now all of node.js's static import syntax is handled by async ops (including something along the lines of promisify(fs.readFile)). that's why i'm requesting that different terminology is used to describe this use case.

requesting that node's loader be synchronous and requesting that it support static import syntax are two very different things, and its confusing if they get mixed up (see my first comment here).

i'm sorry that you feel bullied. i'll try to quiet down a bit.

@benjamingr
Copy link
Member

benjamingr commented May 21, 2018

I’m feeling bullied in this thread. I don’t need to be ordered what terms to use. I also don’t need to accused of bad faith or intentionally misleading people. I don’t find such comments to be likely to lead to a productive collaboration.

First of all - thank you for speaking up! It means the world to me that you're calling this out as you see it. I'm sorry you feel that way. Feel free to reach out to me (at benjamingr@gmail.com) or the moderation team directly (report@nodejs.org) if you would like to discuss how that interaction could have gone better so that all parties can learn how to better interact.

I think this was mostly a miscommunication by both parties due to unclear terminology - I apologize for my part in it and I promise to do better and be more attentive in these situations in the future. If you would like me to self-moderate any of my messages - please do feel free to request it either publicly here or in private in either communication channel outlined above.

I think the issue here is:

This is what I consider to be synchronous syntax:

Neither me nor Gus understood that - ESM imports (even import statements like import { foo } from './bar') are asynchronous generally - one of the things ESM does is allow:

import a from 'b';
import b from 'c';
import c from 'd';

To load the modules b, c and d concurrently (which is unlike what CommonJS does) - so I think the confusion is based on the fact that import statements aren't really "synchronous" the way CommonJS is - they're rather a different part of the script life altogether - happening before code starts executing and establishing live-bindings to exports from modules loaded.

Does that help clear why I was confused about what you were writing?

@ljharb
Copy link
Member

ljharb commented May 21, 2018

@benjamingr the loading might happen asynchronously and concurrently, but the evaluation happens linearly, and behaves as it it was synchronous, in the same tick i believe (iow, before any timeouts or Promise resolutions fire). I think this might be part of the confusion?

@benjamingr
Copy link
Member

@ljharb to be completely honest I think this is confusing because it is actually complicated - things like live-bindings take time to grok and I think it's very hard to see how confusing it is "from the inside". I'm still not sure about some of the terminology myself to be frank.

Just throwing it out there - Maybe we should do a glossary of the different terminology to get everyone on the same page? I may be being selfish here since I personally am sure I'd learn a lot from it.

@bnb
Copy link

bnb commented May 21, 2018

@benjamingr +1 to a glossary. I've been toying with this idea for a blog post for a while (as another extension of this thought) but would much rather see this as an official document. Happy to help contribute 👍

@GeoffreyBooth
Copy link
Member Author

Thanks @benjamingr and appreciate it @devsnek. I would also appreciate a glossary.

If there’s any room for misinterpreting the feature request description then it’s clearly not written well enough. Yes, my intent was that the “synchronous” part only apply to the code that a user writes, not to anything that happens behind the scenes on Node’s side. For the purposes of this feature request, whatever Node does under the hood is irrelevant, if the code I write as a user can be written in a synchronous style.

How about I take one more crack at trying to explain this, and @benjamingr or others, please suggest revisions or alternate versions.


Imagine code like this in current Node:

const { version } = JSON.parse(fs.readFileSync('./package.json'));
console.log(`Running version ${version}`);

This can also be written as:

const { version } = require('./package.json');
console.log(`Running version ${version}`);

Both of these examples are written without asynchronous syntax on the user’s part: there are no callbacks, no promises, no await. Perhaps Node is doing asynchronous stuff behind the scenes, but the user is unaware of it. require appears to be as synchronous as readFileSync.

This feature request is that a user’s code to import JSON files via ESM should be like these examples: no callbacks, no promises, no await. This feature request doesn’t concern Node’s implementation under the hood, and whether it is synchronous or asynchronous; the request is only that the user’s code be able to be written in a synchronous style.

@ljharb
Copy link
Member

ljharb commented May 21, 2018

To clarify, this could also be written as:

const pkg = require('./package.json'); // eg, `import pkg from './package.json';`
const version = pkg.version;
console.log(`Running version ${version}`);

just to remove any distraction about named imports, which are unrelated to this feature.

@benjamingr
Copy link
Member

Though in this particular case since JSON is data and not code we actually could provide named exports 😅

I think we should, by default, regardless of whether or not we do it for CJS. Namely because I don't know of any reasons why not to - am ready to be convinced though.

@targos
Copy link
Member

targos commented May 22, 2018

There's already a discussion about named exports for JSON at nodejs/node#20494

@GeoffreyBooth
Copy link
Member Author

I’ve updated the original post. Please let me know if it can be improved further.

@guybedford
Copy link
Contributor

I think this seems like a subfeature of #116 at this point to me. Let me know if I'm missing anything here.

@GeoffreyBooth
Copy link
Member Author

The syntax part is identical, but the call-out of JSON is different. Perhaps “JSON can be imported into ESM” deserves its own feature.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants