Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

Replace --entry-type with --input-type #66

Closed
wants to merge 1 commit into from

Conversation

GeoffreyBooth
Copy link
Member

Per nodejs/modules#300 (comment), this PR replaces --entry-type with --input-type, a flag just like --entry-type but only for --eval, --print and STDIN.

This way we still provide a way to use ESM in those non-file inputs, but we’re removing the footgun that is --entry-type in its current form. To use ESM in files, the files need to end in .mjs or be in a "type": "module" package scope.

Tests and docs updated.

@devsnek
Copy link
Member

devsnek commented Apr 2, 2019

at this point can we just pr upstream?

@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 2, 2019 via email

@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 2, 2019 via email

@GeoffreyBooth
Copy link
Member Author

What is the solution for extensionless files?

It’s unchanged. As I wrote in nodejs/modules#300 (comment), I’m assuming most people wouldn’t run their extensionless file via node --entry-type=module; for example, you want to just run npm, not node --entry-type=module npm. It defeats the purpose to have a short extensionless filename if you have to run it via a long node command.

So in order to run just npm and for the code of your npm CLI tool to be ESM, npm needs to be a symlink to an .mjs file or a .js file inside a "type": "module" package scope. This is true both under the current --entry-type and this PR. You shouldn’t be putting --entry-type=module in a shebang line because many platforms don’t support that.

@GeoffreyBooth
Copy link
Member Author

Also, I think it is reasonable to work off the fork for changes to the roadmap… So we don’t cause upstream noise when we don’t have consensus on larger changes…thoughts?

👍 We should probably discuss this in the meeting, if people don’t mind. I’m assuming that if this PR gets consensus from our group, I or Myles can open it as a PR upstream. It can’t hurt to get consensus among us first; I feel like our upstream PRs will get accepted more easily if we resolve all our internal disagreements privately first.

@Fishrock123
Copy link
Contributor

I only hope that no one gets confused thinking that this affects process.stdin somehow...

@GeoffreyBooth
Copy link
Member Author

Approved per consensus in 2019-04-10 meeting.

@GeoffreyBooth
Copy link
Member Author

@MylesBorins should I merge this into modules-lkgr or open this as a PR against upstream?

@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 10, 2019 via email

@GeoffreyBooth
Copy link
Member Author

Opened as nodejs/node#27184.

@GeoffreyBooth
Copy link
Member Author

nodejs/node#27184 was merged.

@GeoffreyBooth GeoffreyBooth deleted the flag-for-string-input-only branch April 18, 2019 22:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants