nodejs / CTC

Node.js Core Technical Committee & Collaborators
80 stars 27 forks source link

TC-39 Discussion: Seeking input on import options #131

Closed jasnell closed 6 years ago

jasnell commented 7 years ago

At TS-39, @domenic led a short discussion around the possibility of attaching options to an import statement or import() function. There are a number of use cases for this that need to be considered and there would definitely be an impact on Node.js if this were to move forward. We need folks here to commit to reviewing the issues and providing feedback.

@domenic's presentation is here: https://docs.google.com/presentation/d/1qfoLTniLUVJ5YNFrha7BaVumAnW0ZgcCfUU8UbyyuYY/edit#slide=id.p

Some additional discussion notes are: https://discourse.wicg.io/t/specifying-nonce-or-integrity-when-importing-modules/1861/4

Fishrock123 commented 7 years ago

The syntax is awful.

domenic commented 7 years ago

And yep, I'm out... Good old Node community, welcoming as always.

jasnell commented 7 years ago

@Fishrock123 that's not particularly useful. There are a number of considerations beyond just syntax and it's something that we need to pay attention to. Is there more constructive input you could provide?

jasnell commented 7 years ago

I will point out that tc-39 cares a great deal about this stuff working for node. @domenic has taken great pains to include common node use cases and requirements throughout the proposals discussed. It's worth our time to consider these as carefully as tc-39 is doing. This is stuff that will impact our users for a very long time.

jasnell commented 7 years ago

Fwiw... The conversation landed today on favoring an out of band (non-syntax) driven approach. That said, the discussion right now should be around the needs an be requirements, not knocking down syntax straw people.

Fishrock123 commented 7 years ago

How does out-of-band work for node?

I very much dislike the single "string" approach and would rather:

import "x" with "options"
import("x", "options")
Fishrock123 commented 7 years ago

@jacktuck ... Yes? I have attempted to clarify my initial feedback. Do let me know what is wrong with it.

jasnell commented 7 years ago

@Fishrock123 ... thank you for clarifying.

As this was being discussed, I grokked the use case easily enough but struggled on what would be the most ergonomic approach and unfortunately didn't come up with any really great ideas. One idea that popped in to mind would be to use an options object syntax rather than a string syntax... e.g.

import 'x' with { optionA: 1, optionB: 'abc' };
import('x', {optionA: 1, optionB: 'abc' };

or even...

import { module: 'x', optionA: 1, optionB: 1 };
import({ module: 'x', optionA: 1, optionB: 1 });

In terms of how out of band would work, it would likely be adding to either package.json or having a separate document format.

Fishrock123 commented 7 years ago

I do like the object approach for the import function.

Adding it to package.json poses similar issues as adding the ESM detection mechanism there, IMO.

mhdawson commented 7 years ago

Even with the object import approach, it would have the same problem with "integrity" right ?

The object syntax does seem nicer but I'd prefer the first 2 variants where the options are in an object of their own:

import 'x' with { optionA: 1, optionB: 'abc' };
import('x', {optionA: 1, optionB: 'abc' };
benjamingr commented 7 years ago

Is node even interested in "function import"? As far as I know there hasn't been a built-in way to do an asynchronous require for a while now (for at least a few years) and we've been fine with it.

jasnell commented 7 years ago

If the import function does in fact make it into the language, then it would likely use the same mechanism as regular import. Once we are fully supporting ES6 modules, there won't be a reason not to also support the import function,

benjamingr commented 7 years ago

Once we are fully supporting ES6 modules, there won't be a reason not to also support the import function

Well, it adds to the API surface, it adds functionality that was formerly rejected in core - and the strong obvious use case in browsers doesn't really exist in Node (lazy loading resources and scripts). Every large single page application does chunking and bundling and loading resources lazily while no node server I've seen does anything similar.

I'm not saying we shouldn't use or implement import, I would love to see a convincing use case for the import function first though.

Trott commented 6 years ago

I'm going to close this, but feel free to open another issue in a relevant active repository (TSC perhaps?) and include a link back to this issue if this is a subject that should receive continued attention.