nodejs / node-eps

Node.js Enhancement Proposals for discussion on future API additions/changes to Node core
443 stars 66 forks source link

007: --entry-extension CLI flag proposal #62

Closed bmeck closed 6 years ago

bmeck commented 6 years ago

@rauschma: bikeshed for --input-extension instead

bmeck commented 6 years ago

Mixing anything that is not an opaque text stream such as a filename should error since it doesn't make sense.

matthewp commented 6 years ago

In @ljharb's case I would want/expect the entry-extension to silently override the file extension. If it throws, then

node --entry-extension=mjs app.js

and

node --entry-extension=mjs < app.js

Would do different things, and why should they?

matthewp commented 6 years ago

Ah, I missed @bmeck's reasoning, nevermind. That's fine I think.

rauschma commented 6 years ago

@bmeck: I mostly agree, but executing a .js file as an ES module may be a use case.

bmeck commented 6 years ago

@rauschma pipe it in over STDIN

rauschma commented 6 years ago

@bmeck As that is the only use case I can think of, thatโ€™s probably the right call, yes.

ljharb commented 6 years ago

As long as it's a clear decision one way or the other, I'm happy :-) it just can't be unspecified.

bmeck commented 6 years ago

What happens when a leading . is supplied?

Stream goes through CJS require.extensions then ESM import mechanics. It wouldn't find a ..js etc. extension and would fail.

What happens when an unknown extension is supplied?

Fails to load.

Is there a way to list supported extensions on the CLI?

Not currently, but could be proposed.

bmeck commented 6 years ago

This change does not attempt to change workflow of how files are loaded. It just declares the mode rather than going through CJS always. See https://github.com/nodejs/node/blob/5fd2f03b16bfd6730fe82dc3217f1d05131b9615/lib/internal/bootstrap_node.js#L435 for some existing implementation details.

ljharb commented 6 years ago

It seems like node --entry-extension=foo should fail immediately if foo isn't in require.extensions, and ideally would print out Object.keys(require.extensions) in the error message.

bmeck commented 6 years ago

@ljharb I am hesitant to do that error behavior since people could certainly add to the list using --require. Also, require.extensions is not used in the ESM logic.

ljharb commented 6 years ago

@bmeck in that case i'd expect the list to be runtime-computed, so if they'd added to the list, it would become a valid option.

However it's implemented, I think it'll be important to fail-fast when an invalid extension is supplied, and make sure the error message is meaningful and actionable.

MylesBorins commented 6 years ago

@bmeck do you think it makes sense to close this as we are no longer using the eps process?

bmeck commented 6 years ago

@MylesBorins yes, as I presumed all EPs would be considered archived I didn't touch it.

styfle commented 6 years ago

007 is dead

007

See what I did there? ๐Ÿ˜„

......I'll show myself out now ๐Ÿƒ ๐Ÿšช