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

upstream objection to --type #296

Closed
MylesBorins opened this issue Mar 18, 2019 · 51 comments
Closed

upstream objection to --type #296

MylesBorins opened this issue Mar 18, 2019 · 51 comments
Assignees
Milestone

Comments

@MylesBorins
Copy link
Contributor

specifically that the name of the flag --type is too generic. I've renamed it --entry-type. There is also the question regarding if it should be scope to entry point or be an alias to override package.type... basically the exact conversation from the last meeting

Latest comment nodejs/node#26745 (comment)
Implementation nodejs/node@59a4ef8

@GeoffreyBooth
Copy link
Member

So we still have the issue raised in nodejs/ecmascript-modules#57 that basically, a flag that sets the module format of only the entry point introduces an issue where node --entry-type=module entry.js can import './file.cjs' which can then require('entry.js') and now we have entry.js loaded as both ESM and CommonJS. I don’t think an entry point-only override can work, and I also don’t see the use case for it (versus the flag setting or overriding the "type" field for the scope that the entry point is in).

I propose we rename the flag to --package-type and it sets or overrides the controlling package.json "type" field for the entry point’s package scope.

@MylesBorins
Copy link
Contributor Author

I'd like to suggest the following behavior

  • --package-type: if this flag is used it sets the type and overides the package.type field if it exists. If subsequent package.json's are found in children they are respected per the algorithm... e.g. it only overrides the base package.json
  • if there is no --package-type the package.type field is used next
  • if there is no package.type the default is commonjs

@ljharb
Copy link
Member

ljharb commented Mar 18, 2019

At the moment, we only have consensus for a flag for the entry point - I agree that if it were a mode, and matched the type field, package-type would work.

@MylesBorins
Copy link
Contributor Author

@ljharb I believe that we would have had consensus for this in the last meeting if you didn't object.

Are you +1 with moving forward for now? We can implement and have one more meeting for the team to review and +1 final implementation before landing

@ljharb
Copy link
Member

ljharb commented Mar 18, 2019

with extending the scope of the flag to be a mode? no, I'm still not convinced a mode like that (especially settable via a NODE_OPTIONS environment variable) is a good idea for the larger ecosystem.

@devsnek
Copy link
Member

devsnek commented Mar 18, 2019

i thought we voted and package scoped won

@ljharb
Copy link
Member

ljharb commented Mar 18, 2019

As I recall, the one thing we voted on was whether .cjs was a blocker or not; and "keep .cjs" won.

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Mar 18, 2019

@ljharb can you expand on why please? I'd like to see this objection combined with a more in depth reason to object aside from "not a good idea".

I personally find the current behavior to be confusing and the ecosystem response to the current flag seems very much to be in line with the expected behavior that has been voice by multiple members of this team.

Up until this point we've been pretty good about respecting any objection, but now that we are moving upstream I think that objections need to come with a very strong and solid reasoning. In core, people cannot just "object" they need to have a strong case or that objection will be overruled, generally by the TSC.

Lone objections being unwilling to compromise will block our ability as a group to be chartered, the TSC will not want to charter a group that consistently requires their intervention.

edit: and I am unwilling to charter a group that consistently requires voting to handle objections

@WebReflection
Copy link
Contributor

WebReflection commented Mar 19, 2019

As previously mentioned, I believe --type=module|commonjs is the best way to go, and I don't see any compelling argument to define it ambiguous or inappropriate anyhow, specially because the type keyword was chosen to mimic the <script type="module"> Web behavior, making --type also future-friendly whenever new type could land on browsers.

However, if --type cannot be used, I'd like to discuss the alternatives I've read so far.

--package-type

This is super semantic and a no-brainer, but only if you think about the package.json field, otherwise it doesn't make much sense.

Example:

node --package-type=module standalone

I guess we agree there is no competition against node -m standalone, or even node --type=module standalone, specially when there is no package.json around.

--entry-type

Semantically speaking, entry and type combined make a lot of sense, but only in a CLI context.

If the package.json counter part is just type, instead of entry-type, having to remember the CLI switch VS the package one, is IMO unnecessary, inconsistent, overhead.

About -m

Even if this shortcut is limited to the module type only, it's pretty consistent with the rest of the CLI based ecosystem (SpiderMonkey / JSC) so, if there will be any shortcut, I think -m should be considered as special case over alternatives such as -t module or -et module

Expanding on entry-type opportunity

What I see as a missed opportunity with the current MR/state, is the ability to publish dual modules, as it's been done for the last couple of years.

The community already adopted the module field, and kept the main one reserved for CJS.

Various packages in npm are indeed published as such:

{
  "main": "cjs/index.js",
  "module": "esm/index.js",
  "unpkg": "min.js", // or "cdn" or others ...
  ...
}

If the current proposal is to use type as field to specify how the main one should be loaded, every package currently published as dual module won't be able to keep working as such (dual).

However, if we ask npm developers, AFAIK still part of this group too, if it'd be OK to have an entry-type field reserved in their package namespace, we could create a future-proof, present-ready, backward compatible, possibility to define any entry.

Example:

{
  "main": "cjs/index.js",
  "module": "esm/index.js",
  "entry-type": {
    "module": "esm/index.js",
    "commonjs": "cjs/index.js"
  }
}

Shortcutting as type (less ugly alternative)

If the previous idea would make people unhappy, or even better, if we keep the --type flag, how about asking npm folks if it's OK for npm to also trust/reserve a commonjs field?

{
  "main": "cjs/index.js",
  "module": "esm/index.js",
  "unpkg": "min.js", // or "cdn" or others ...
  "commonjs": "cjs/index.js"
}

The module field is somehow de-facto reserved in the package.json file, so how about we use --type to define which field of the package should be used?

As example, let's imagine a standalone file contains the following:

import module from 'package';

Now, there are three ways to run such file

# run standalone as commonjs
node --type=commonjs standalone

# run standalone as module
node --type=module standalone

# just run standalone
node standalone

The following algorithm, assuming the imported package has a related package.json, would apply:

[ edited few times, sorry ]

  • if standalone is run as commonjs or without any type (default)
    • if there is no type field within the package.json
      • if there is a commonjs field within the package.json, require such file
      • otherwise require the main field
    • otherwise use the type information
  • else if standalone is run as module
    • if there is no type field within the package.json
      • if there is a module field within the package.json, import such file as ESM
      • otherwise import the main field
    • otherwise use the type information
  • else throw an "unsupported type" error

Above flow would always be able to disambiguate imports of any kind, keeping the ability to publish dual modules as it's been done until now, and as already managed by pretty much all bundlers.

Example of fully future/backward compatible package:

{
  "main": "cjs/index.js",
  "module": "esm/index.js",
  "unpkg": "min.js", // or "cdn" or others ...
  "commonjs": "cjs/index.js",
  "type": "module" // or "commonjs"
}

Above package would work in all current bundlers, providing entry points for both ESM and CommonJS, it would work in Node < 10 (or whatever version won't receive the --type flag), it won't require any maintenance for currently published dual modules, it'll always disambiguate through type when needed, it still offers fallbacks for CDNs or good old browserify.

Please note, I'm not saying people should produce dual bundles from now on, I'm saying these kind of packages has already landed in npm, and while things settle down with the new ESM module system, it might be handy to have a migration pattern that won't break the current ecosystem.

As Summary

I personally think keeping --type is the right thing to do, but I also would like to see such type reflected, and overruling, when appropriate, the main field, as that's been historically related to CommonJS, and specially in node.

Hope something I've written here could be considered, happy to answer questions, if any 👋

@GeoffreyBooth
Copy link
Member

Reading through the comments on the upstream PR, it seems that everyone assumes that setting the type of the entry point therefore tells the project to operate in that mode. As in, they get that --type sets the mode of the entry point—but then they assume that therefore, .js files imported from an ESM entry point will be treated as ESM. @WebReflection above also makes this assumption.

So even if we rename the current feature --entry-type, a huge proportion of users will assume that that sets the type for their project, or rather for all files referenced in a tree starting from that entry point.

I can’t think of a use case for when a user might want to set the entry point type and not have that affect all the files in that scope. I can think of a use case where a package author might want an ESM entry point (to define named exports) for an otherwise CommonJS package, but a) packages aren’t run via flags, and b) there wouldn’t be a reason for a user to use a flag to override such a package/project. Absent a use case for the current behavior, I see no reason not to change it to the proposed package scope behavior and make the flag conform to user expectations (and use cases).

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Mar 19, 2019

Some names, please emoji vote

--type 👍
--entry-type 😁
--package-type 🎉
--mode 👀
--module-mode ❤️
--import-mode 🚀

@ljharb
Copy link
Member

ljharb commented Mar 19, 2019

@MylesBorins given the way all the things that are in the upstream PR were sold to me and the working group, moving the bar for objections now seems highly deceptive. Many parts of the implementation were landed with "we can just revert them later!".

Again, we do not have consensus for a mode, but we can certainly add that to the next meeting's agenda. While it's not a mode, entry-type is my choice; if it were a mode (which it's not), package-type makes the most sense to me.

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Mar 19, 2019

@ljharb it was clearly stated from the get go that upstream may object to changes and we would need to adjust. Calling this deceptive is imho disingenuous. I would like us to try and work through as much of this as possible in advance of the meeting.

You mentioned you have objections to the mode, and afaict right now are the only objector, can you please expand on what your issues are?

Keep in mind, the modules team is not chartered. The TSC and the project has been kind enough to respect our request for a moratorium, but we have no explicit say on things... we can only make suggestions. There are no guarantees about what can and can't land and we are in no place to do anything but make recommendations. If we have to wait 2 weeks in between each set of objections or feedback from upstream we will not get things done in time. There is no other team or working-group within the foundation that works on that cadence, we need to be able to iterate faster.

@SMotaal
Copy link

SMotaal commented Mar 19, 2019

Since we are upstreaming, I really think we all should do our best to give priority to focusing our efforts on concerns addressed to us as a team — and while we all have things we object(ed) on all the time, it really only ever matters to bring those up when they are in the teams best interest (ie during times where not raising them would result in subpar implementation work.

I ask everyone on the team to appreciate that our differences make us deliver our best efforts, but not to forget that some among us have put a lot of hard work to meet upstream obligations on behalf of the team, and aside from me (mostly) when this upstream pans out (even if we need to address things moving forward) it is only credit that reflects the lengthy involvement and dedication to respond to demands from the community on one of the most fundamental aspects of almost every single package out there.

If concerns need to be voice, please use doodle, get us together and talk it out. If concerns need to be addressed, please find a good way to represent our team, and appreciate that the team respects your opinions and want to find ways to work towards making them happen.

@joyeecheung
Copy link
Member

joyeecheung commented Mar 19, 2019

To elaborate on --type itself (this applies to flags added for any feature, so it's not really related to modules):

We already have very verbose flags in core, and most of our flags are pretty verbose up until this point. The existing non-experimental short flags are pretty old and they are in the minority - those can't really be changed now out of compatibility concerns. The recently added flags are all pretty verbose (e.g. the --diagnostic-report-* ones), and if not, they are at least not too generic (e.g. it's not too difficult to guess at least what subsystem --inspect and --loader are for). So that leaves us three choices when we graduate new flags out of the experimental status:

  1. Use both verbose flags like --diagnostic-report-uncaught-exception and succinct but ambiguous flags like --type:
    • Pros: ?
    • Cons: lack of a coherent style. As the flags are part of the API contract, it leads to a sense of lack of design of the entire runtime.
  2. Keep the new flags verbose (or at least not too ambiguous) from now on:
    • Pros: unambiguous
    • Cons: difficult to type
  3. Keep the new flags succinct from now on:
    • Pros: easy to type
    • Cons: ambiguous, and is likely to lead to conflicts eventually.

@WebReflection
Copy link
Contributor

@joyeecheung most of the users won't probably ever write --diagnostic-report-* once in their node experience, while all of them will write --type=module at some point.

Short flags are simply handy, in the history of software, and I consider node -e as successful as, potentially could be, node -m.

If the shortcut -m to represent --type=module is too much, at least keep it simple to type and remember for everyone that understand how script type=module works.

@GeoffreyBooth

they get that --type sets the mode of the entry point—but then they assume that therefore, .js files imported from an ESM entry point will be treated as ESM.

what would be the meaning of type otherwise if not to specify the package or target file mode scope?

When you enter into ESM on the Web there's no way to unstrict from it, unless explicitly hooking yourself into Function evaluation.

If .cjs made it, which can always be used to disambiguate that pose goal, what would be the reason to not have type define how the code, from that moment on, should be executed?

I don't see any useful, real-world, use case neither, and it'd be absolutely unexpected if once forced ESM, an import would consider .js like legacy CommonJS files.

If that's meant/needed, then .cjs will fix any possible use case (an intermediate file to load anything from CommonJS doesn't look like a bad thing at all).

@devsnek
Copy link
Member

devsnek commented Mar 19, 2019

@WebReflection "most of the users won't probably ever write --type=module once in their node experience, while all of them will write --diagnostic-report-* at some point."

Please don't make baseless assertions. we have absolutely no idea how often usage any of these flags are actually used (although we do already know that node-report and friends are super popular for debugging).

@SMotaal
Copy link

SMotaal commented Mar 19, 2019

@joyeecheung Thanks for articulating the specifics, which helps us better frame the right questions to get us in the right direction.

a. Certainly, verbose flags are essential for things that are ambiguously problematic, ie if more than one aspect (or subsystem) have overlapping configurable parameters. Q where else would we consider --*-type?

b. Utility and priority always take center stage from a UX perspective. From the command line, all things are flags and then you have the entrypoint. From a utility perspective, you need to affect the entrypoint and so you will use a flag and you have --type=module entrypoint because you just wanted to quickly exec code you wrote and not set up to fiddle with what package scope is or where to get one 😉 — Q would it be reasonable to consider verbosity on things utilized more systematically of higher priority than things utilized incidentally when they are intended only to be used "directly" compared to the more scriptable uses?

@WebReflection
Copy link
Contributor

@devsnek I'm making assertions based on the team I've worked with, where these flags where either ignored, or written in bash files from ops, not from developers using node for either tooling or backend.

But if there is a type to switch mode, same way every dev at some point wrote, or understood, the "use strict" directive, I believe everyone will at some point write, and understand, --type=module or --type=commonjs.

Having short names for flags is easier to both type and remember, as already mentioned by @joyeecheung too

( I know tar went too far, but I'm talking flags, not short flags 😅 )

@joyeecheung
Copy link
Member

while all of them will write --type=module at some point.

Why? Aren't -m and the package.json field enough for most use cases? Shortcuts are exceptions as they are not even really names, you'll have to look at the docs anyway.

Separately, even if we are going with single-worded names, the flag --type=module itself is not really obvious for what it means for me (I guess --entry=module would at least work better than that). I can't really connect this with <script type="module">, shouldn't it be --script-type if we are going with that line of thinking (even though it's a pretty incorrect name)?

@MylesBorins
Copy link
Contributor Author

@joyeecheung I think the idea was that there may eventually be multiple modes e.g. wasm.

So if you wanted to use a wasm entry point without a file extension we would need --mode=wasm

@devsnek
Copy link
Member

devsnek commented Mar 19, 2019

this all hinges on --package-type=xxx being too difficult to type. i think that's a rather subjective thing to begin with. but regardless of the name, people like @WebReflection who will use this constantly can use aliases or environment variables. people who don't use it often will benefit from a more specific name.

@SMotaal
Copy link

SMotaal commented Mar 19, 2019

@joyeecheung --type=module is chosen because of its symmetry with <script type=module> and that is meant to minimize the cognitive burdens for less seasoned users who have already experienced this divergence from type=application/… (ie mimes) to a simpler type=module.

@devsnek fyi, there is a lot more to it than being too many --package-type=module

@MylesBorins
Copy link
Contributor Author

this all hinges on --package-type=xxx being too difficult to type.

@devsnek I think it might be more about it being a PITA, and that people may want to type it semi-regularly.... although that might be much more an argument for -m... although that really doesn't scale to many modes.

@WebReflection
Copy link
Contributor

@joyeecheung

it's script that gives it context.

On a Web document, the script tag is the directive that tells the browser to interpret, and execute, the either inline (evaluated) or remote (loaded) content.

Accordingly, script type=module on the Web, is the exact equivalent of node --type=module on any OS: it's the explicit intent from a developer to load/run/execute some content through a specific type.

@SMotaal
Copy link

SMotaal commented Mar 19, 2019

@joyeecheung thanks for the added clarity, I think that these finer points of discussion are ultimately what it takes for us to find the most reasonable path out of "experimental" for the community at large.

Would it be reasonable to consider that with this being --experimental-modules related that we get the pulse of those in the community who are consciously taking this all out for a spin to a) point out if and how it was an issue for them or b) use a mid-way survey to give feedback on which option makes more sense after use it?

@SMotaal
Copy link

SMotaal commented Mar 19, 2019

Separately for a design standpoint, we need to also worry about one possible future scenario where one may have some 2018 Node.js projects that they want to use and now they will need to use --type=commonjs to get their Node.js to actually not throw.

So, our priorities:

  1. Make sure it aligns with overall Node.js style

  2. Make sure the end-user does not find it counter-intuitive "today"

  3. Make sure we don't end up making it more difficult to address decisions we cannot predict today (this ironically always happens because of 1 or 2).

So at this point, all our best arguments cannot concretely address anything beyond the first point. My opinion would be to get a small representative sample for the second one and do our best to make the best call on the third.

@WebReflection
Copy link
Contributor

some 2018 Node.js projects that they want to use and now they will need to use --type=commonjs to get their Node.js to actually not throw

how so? If I understand correctly --type=commonjs is the implicit type unless the entry point has an explicitly .mjs extension, isn't it?

@joyeecheung
Copy link
Member

joyeecheung commented Mar 19, 2019

On a Web document, the script tag is the directive that tells the browser to interpret, and execute, the either inline (evaluated) or remote (loaded) content.
Accordingly, script type=module on the Web, is the exact equivalent of node --type=module on any OS: it's the explicit intent from a developer to load/run/execute some content through a specific type.

This mental models seems a bit hard to follow to me - in a Web document, the code in a <script> tag may share the memory with the code in the previous <script> tag, but it's not the case for CLI programs. Two scripts executed by node consecutively do not share the same context.

@WebReflection
Copy link
Contributor

@joyeecheung the point is the context of execution ... where script is replaced by node. I also think you are not willing to see --type the same way many see it already so I won't insist.

@joyeecheung
Copy link
Member

joyeecheung commented Mar 19, 2019

Would it be reasonable to consider that with this being --experimental-modules related that we get the pulse of those in the community who are consciously taking this all out for a spin to a) point out if and how it was an issue for them or b) use a mid-way survey to give feedback on which option makes more sense after use it?

This could be a totally bad idea but...since this is going to be an experimental flag, can we display a warning for --type with a link and ask the user to do a survey?
(well we could do it for any flags that are part of ESM really, the user can use all sorts of configurations like NODE_NO_WARNINGS=1 to silence the warning, or we could add some special switch just for this)

@SMotaal
Copy link

SMotaal commented Mar 19, 2019

@MylesBorins can we consider the calculus on us having a feedback structure baked in from the get-go?

@GeoffreyBooth
Copy link
Member

This could be a totally bad idea but…since this is going to be an experimental flag, can we display a warning for --type with a link and ask the user to do a survey?

We absolutely could. Currently every time the --experimental-modules flag is used, a warning is already printed:

ExperimentalWarning: The ESM module loader is experimental.

That could easily display more text, including a URL, if the Node core team approves. We could also limit the additional text to appear only when --type is used.

@SMotaal
Copy link

SMotaal commented Mar 20, 2019

@GeoffreyBooth so how about the means for which we would set it up so that this URL provides a well controlled way for reliable feedback (ie both UX and sampling validity considerations) — thoughts?

@devsnek
Copy link
Member

devsnek commented Mar 20, 2019

i don't believe a poll is the correct way to design features in node (or really anything). a user of node has no obligation or intensive to think about node's design choices and probably is not well versed in our roadmap and intentions. that's why we have groups of people who are invested in understanding and driving node's forward movement.

@SMotaal
Copy link

SMotaal commented Mar 20, 2019

The question to be had is if it would be reasonable UX wise, and that is where the user tells us, the flip side is what aligns, and this is a separate consideration per comments.

@devsnek please review my earlier comment — Also, re point (1) this was what the focus of everything we did as a team leading up to last week's meeting.

@GeoffreyBooth
Copy link
Member

In my notes from our last meeting, the only reasoning given by @ljharb as to why he objected to a package scoped --type flag was that he “wanted users to need to change their files,” presumably to encourage adoption of .mjs and the "type" field.

Can I suggest that maybe this desire be amended slightly to be “wanting users who publish to the npm registry to need to change their files”? Because if this is acceptable, then either the current or the proposed package scoped --type flags are acceptable, because they never apply to a project’s dependencies. By definition, the flag only affects a user’s project; any subfolder of node_modules would have its own package.json and therefore be unaffected by the flag, which means that any author of any package published to the npm registry would need to use .mjs or "type" in order to use ESM syntax in their Node-compatible package that they want to publish.

@jkrems
Copy link
Contributor

jkrems commented Mar 20, 2019

A package scoped --type CLI flag seems like a pretty problematic idea to me. So I would object to that as well. It's asking for a bunch of confusion because it will break every step of the way. It's --harmony all over again - your test runner / debug wrapper / ... all would need to start supporting this flag and do extra forks to apply it. I'm +1 to an explicit --entry-type flag that applies to exactly the file/source passed to the CLI. I'm -100 to a flag that changes the global state of node and propagates to other file loads/executions, even if it is limited to an implicit directory boundary.

@GeoffreyBooth
Copy link
Member

your test runner / debug wrapper / … all would need to start supporting this flag and do extra forks to apply it.

This is more true of a flag that affects only the entry point, because then these loaders you’re describing need to check if the entry point is incongruent with the "type" of the scope that it’s in. The proposed --entry-type=module would set an entry .js file to be ESM while all the other .js files in that scope remain CommonJS.

Whereas if the flag is just a proxy for "type", which such loaders may need to understand anyway, then they only need to check one thing. We could provide an API for getting the type of a particular scope, and Node could handle the logistics of whether that type was set by a flag or the field.

The bottom line is that we need --type, because without it there’s no way to support ESM syntax in non-file inputs like --eval and STDIN. Users will expect --type to work not just with those modes but with files as well, and when it works with files it should behave as users expect. Users expect it to behave like the package scope "type" field.

@WebReflection
Copy link
Contributor

Before I write anything at all, can anyone please describe what does "type": "module" mean in a package.json file?

Assuming I understand that means the scope is ESM so that files with a .js extension in that module would be imported as ESM, why would the --type=module flag be any different, to explicitly opt in into ESM for an explicitly passed file/entry that will assume anything it loads as .js should be imported as ESM as if it was executed through a "type": "module" package?

@GeoffreyBooth
Copy link
Member

@WebReflection The new docs explain it pretty well.

@jkrems
Copy link
Contributor

jkrems commented Mar 20, 2019

Let me clarify: The above isn't meant to block upstream progress because I'm assuming we can always iterate after getting it upstream. But I would vastly prefer renaming the flag to extending its scope. It's especially useful when there is no package.json which implies that there's either no boundary or a weird implicit boundary. It feels super magical and not intuitive at all (especially in cases where it may jump in and out of the import loader, e.g. via dynamic import in Common JS).

@jkrems
Copy link
Contributor

jkrems commented Mar 21, 2019

To clarify further: For me the first priority is to land upstream. From then on we can keep iterating and have further discussions. It's all still malleable (even if we need to stop bikeshedding eventually). :)

@WebReflection
Copy link
Contributor

WebReflection commented Mar 21, 2019

@GeoffreyBooth thanks.

Accordingly, if this flag is supposed to mimic exactly the same behavior, I'd go for --package-type=module so that nobody can say it's ambiguous, 'cause it'd do exactly what a parent level package.json file with a "type": "module" field would do: it enables ESM and imports files as ESM, unless the extension is explicitly .cjs.

Any other behavior or name that won't mimic that type concept would create unnecessary confusion, imo.

@SMotaal
Copy link

SMotaal commented Mar 21, 2019

@WebReflection

how so? If I understand correctly --type=commonjs is the implicit type unless the entry point has an explicitly .mjs extension, isn't it?

I meant in the far future, where one possibility would be that in 2018 the default and only js thing in Node.js was cjs and in that future who knows 🔮

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

No branches or pull requests

10 participants