nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.53k stars 29.03k forks source link

import.meta.main #49440

Open MylesBorins opened 5 years ago

MylesBorins commented 5 years ago

Deno just added this.

Should we?

https://github.com/denoland/deno/pull/1835

ljharb commented 5 years ago

I'm a bit confused what it does - does it mean "it was the main in package.json"? it was the primary entry point? what about import()?

devsnek commented 5 years ago

it's true if the file was the entrypoint. for node i'd rather it be called something else cuz the term main already has package-scoped meaning.

ljharb commented 5 years ago

Right, but anything that's import()ed is also an entry point, and I'm very unclear on why you care what that value is. Currently you can look at process.argv and __filename to determine that (and import.meta.filename or similar is quite well motivated).

Also yes, main is a terrible name for multiple reasons, not the least of which is that it's a package.json field.

guybedford commented 5 years ago

I really like the import.meta.main concept. Currently in Node.js it is common for CLI tools to do the following check:

if (require.main === module) {
  console.log('This is a CLI tool! Usage: ...');
}

We currently have no version of the above check in ECMAScript modules, so this is a problem that comes up, and there is currently no easy way to do this without it feeling like a hack.

As an already-paved use case, users are familiar with the main terminology in this context already.

The above check then becomes:

if (import.meta.main) {
  console.log('This is a CLI tool! Usage: ...');
}

and the really nice thing about this pattern is that it works in browsers and other environments completely fine without any need for compatibility API layers or compilation.

For these reasons I'm 100% supportive of import.meta.main.

ljharb commented 5 years ago

Doesn’t that use case predate the current community preference, which is to have separate packages for a library and a CLI?

guybedford commented 5 years ago

@ljharb this use case is explicitly documented in https://github.com/nodejs/node/blob/master/doc/api/modules.md#accessing-the-main-module.

ljharb commented 5 years ago

Sure but so is every aspect of the require algorithm :-) the better question is, is it still a common or desired use case to determine if something is the entry point, and how does that change now that dynamic import allows multiple entry points?

guybedford commented 5 years ago

@ljharb yes it is a desired feature. The require.main === module was something we had to explicitly support in the ncc project as users were using it in CLI tools. This came up pretty early in the project and had multiple users asking about it here - https://github.com/zeit/ncc/issues/224. Dynamic import does not affect this, as what we are distinguishing is the CLI entry point, not the module graph entry point.

ljharb commented 5 years ago

The feature seems fine then (it’d only be true in the top-level process entry point); the name, definitely not (but we can bikeshed that).

bmeck commented 5 years ago

Does this absolutely need to be in our first iteration or can we get feedback / focus on other things?

zenparsing commented 5 years ago

Thanks for bringing this up @MylesBorins. I (think) we can say this is post-MVP but it's good to have on our radar.

MylesBorins commented 5 years ago

can definitely be post MVP. Just wanted to document what's going on in ecosystem

shian15810 commented 5 years ago

Well, I'm not part of the organization, but as a cli maintainer, this proposal seems so interesting.

Can I start to try implementing this in https://github.com/nodejs/node and possibly a PR now?

devsnek commented 5 years ago

I'm not a huge fan of the concept of main. imo you should have separate files for bin and lib, which is something other languages do just fine, and installers like npm and yarn already let you specify separate bins.

guybedford commented 5 years ago

@shian15810 thanks for showing an interest in the modules work! Contributions are very much welcome to Node.js core and the module work here.

We do have some consensus issues for this feature due to our not wanting to provide unnecessary features without seeing a strong need for them in the initial modules implementation. import.meta is a very widely used namespace (every module!) with strong backwards compatibility needs, so we do also need to be very cautious about what we put on it.

A PR to Node.js core would certainly drive discussion, and may even sway consensus. Also hearing more about your use case as a CLI maintainer and how this is useful could help us better understand the importance of the feature to you (including for example the points raised by Gus above). Even if the PR sits without approval, we may be able to come back to it in a few months even as well.

shian15810 commented 5 years ago

The use case of mine is that both bin and main fields in package.json point to the same file. Using require.main === module, as mentioned in the official docs here, to tell whether the script has been run directly or not, so that to know if the main function should be executed or exported.

As far as I can tell, with a package.json having "type": "commonjs", there are three ways to tell if a file is run directly (aka not being required):

  1. require.main === module or process.mainModule === module
  2. require.main.filename === __filename or process.mainModule.filename === __filename
  3. !module.parent

However, with a package.json having "type": "module", require and module are not available, as well as process.mainModule is undefined.

What's left is just import.meta.url, but the problem is what to compare with to know if it is the main script.

In addition, according to this, module.parent in a cjs script will be undefined if the parent is a mjs script. After some testing, require.main and process.mainModule are both undefined too in this context.

In my imagination, if import.meta.main is implemented, I could use it in mjs script, while retaining require.main === module in cjs script. But this still pose a serious problem to a project with mixed cjs and mjs scripts importing each other, as mentioned above.

So yeah, I agree with @devsnek in this case, that is pointing bin and main to separate scripts in the first place, which I now think is the right way to solve this problem.

Also, I agree with @guybedford regarding strong backwards compatibility, even though TC39 states that import.meta object will be extensible.

As a last note, I've read somewhere in this repo stating that in the context of esmodule, there is no concept of main such as in commonjs, correct me if I'm wrong. But I certainly think that there must be some other use cases of determining the main script, just like require.main === module in commonjs. It is good to have import.meta.main as the counterpart in esmodule, though should it be named main is pretty much debatable.

MylesBorins commented 4 years ago

Going to abandon this for now, please feel free to reopen

guybedford commented 4 years ago

I still believe this is an important feature, to be put in the same basket as CJS features not available in ESM like require.resolve.

It's not a common feature, but it is definitely a heavily ingrained one where it is used, and has a small collection of users that absolutely do expect this functionality (mainly CLI developers).

devsnek commented 4 years ago

i am still against the concept of a main module in general.

SMotaal commented 4 years ago

@guybedford Which labels are relevant here? Can you add them to make it easier to related to the context please — ie cjs anr/or esm... etc.

SMotaal commented 4 years ago

@devsnek Can you elaborate more? Main here is the main entrypoint, so it is like window.location where there is a Window context or self.location where there is not (workers... etc.) but that does not apply the same way in CommonJS (ie not URLs).

Aside — I don't think adding a global for ESM only is a good idea personally, and otherwise neutral to import.meta.main.

SMotaal commented 4 years ago

@guybedford Awesome, that really helps 💯

devsnek commented 4 years ago

@SMotaal i dislike the idea of differentiating entrypoints. you can just use a separate file for your bin.

devsnek commented 4 years ago

being an entrypoint doesn't inherently mean it's a cli (for example, cf workers). additionally, some hosts like browsers don't map well to the concept of entrypoints. depending on how you think about it, a webpage may either have several or no entrypoints. Even if you choose to think about a browser having several instead of none, none of the several would be a cli.

SMotaal commented 4 years ago

@SMotaal i dislike the idea of differentiating entrypoints. you can just use a separate file for your bin.

@devsnek certainly it is one style, I am not saying your style is wrong, but others also have justifications that cannot be open to judgement because this all comes down to matters of opinion.

As it happens, any given module in a package can be called as a result of at least two paths, a main entrypoint in the package, or an entrypoint for which the package is a direct or indirect dependency… some packages may actually consider this a functional parameter depending on their purpose.

depending on how you think about it

Yes, I think this is the bottom line… And, so no disagreement or disapproval is intended, just wanting us to consider the more diverse landscapes that we may not ourselves be interested in (yet).

guybedford commented 4 years ago

To clarify some of the above, the only module that gets import.meta.main set is the module passed into node main.js it is only the module corresponding to process.argv[1]. And no, it doesn't apply to browsers.

This is the Node.js application entry point main, not package entry points, or realm / worker entry points.

An equivalent way to achieve this check would be to do path.resolve(process.argv[1]) === fileURLToPath(import.meta.url).

Perhaps the above explicit check would be enough though.

devsnek commented 4 years ago

@guybedford what i'm trying to say is that because it depends on how you approach it, we shouldn't try to reify a certain pattern. if the condition for your cli is that it is the argument to node, that check seems good to me. i agree it's not the most ergonomic, but i think that comes down to the awkardness of paths and urls.

SMotaal commented 4 years ago

@guybedford thanks for the reminder/clarification… so a consideration that comes to mind, who has the authority to know.

And so it being a "readonly" and "private" field on import.meta has very different implications than globals, where it is possible to run into all kinds of costs to try to check for distortions.

Just a thought, no strong feelings on it personally.

guybedford commented 4 years ago

The existing Node.js pattern it is replacing is the require.main === module pattern which is documented in the Node.js docs for require - https://nodejs.org/dist/latest-v13.x/docs/api/modules.html#modules_accessing_the_main_module

devsnek commented 4 years ago

the pattern for cjs, sure. i'd argue there is no pattern for esm yet. i'd like to lean on the more mature advancements in the ecosystem like package manager bin entries instead of continuing to copy old python patterns. using separate files also means (as an example) that node-specific cli code is not included if you just load up a library for webpacking.

guybedford commented 4 years ago

The point for keeping this open is only that this falls under things supported in CJS but not ESM right now and that there are many users who would expect this functionality. That doesn't mean we have to do it, but only that we shouldn't neglect those existing user expectations.

devsnek commented 4 years ago

right, i'm saying we should put a section in the docs saying "for those of you used to including your lib exports and bin logic in the same file, the esm pattern is to use two separate files". i'd be happy to write it up too if this discussion finishes in favor of that.

addaleax commented 4 years ago

@devsnek I find it curious that you’re both saying “we shouldn't try to reify a certain pattern” and “the esm pattern is to use two separate files”.

I heavily agree with the first sentiment, which is why I feel we should not enforce using two separate files rather than a single one.

bmeck commented 4 years ago

I don't think we are explicitly forcing using two files as we have pointed out a way of doing this, and other workarounds do exist. I think the using of two files is just a natural feeling solution compared to other workarounds and is why people think of it as a specific pattern to use. I personally remain unsure of exposing a new property on import.meta to fill this feature; workarounds already exist so perhaps we can lean on those until the need to expose something proves to be urgent.

jkrems commented 4 years ago

There's also the issue with block-scoped loading of dependencies. In CJS require.main was nice because it allowed inlining things like:

if (require.main === module) {
  const assert = require('assert');
  const http = require('http');

  // Setup example and/or test
}

In ESM, that pattern either needs to use an awkward await import('assert') or, more likely, importing those test/example dependencies at the top of the file. I wouldn't be too eager to add import.meta.main unless we see people use one of the workarounds described above in "real" code. If isMain(import.meta) ends up a popular npm package, we can always add the property.

devsnek commented 4 years ago

@addaleax good point :) To clarify: I was approaching it from the perspective of what people would do if node doesn't add anything special for this case. Like guy said, there are more patterns, like checking argv. Maybe it would be best not to document anything specifically though.

azz commented 4 years ago

Just got blocked on this when switching some apps to use native ESM. We have the same, very common pattern of:

function main() {}

if (require.main === module) {
  main();
}

Replacing this with if (import.meta.main) {} would be very desirable. It's actually an improvement on the CJS version.

devsnek commented 4 years ago

@azz is using separate files not acceptable for some reason? more info would be good.

azz commented 4 years ago

Moving to a separate file is fine in some cases, but can cause some tricky refactoring if there are dependants on the existing behaviour. It can be a breaking change.

azz commented 4 years ago

Another use case is scripts/CLIs that are distributed as a single JS file.

tschaub commented 4 years ago

This would be a pretty nice upgrade path for people wanting to use modules with Node:

--- a/nice.js
+++ b/nice.js
@@ -1,4 +1,4 @@

-if (require.main === module) {
+if (import.meta.main) {
   main();
 }

I'm glad to know about this workaround, but this is more awkward in my opinion:

--- a/not-as-nice.js
+++ b/not-as-nice.js
@@ -1,4 +1,6 @@
+import {fileURLToPath} from 'url';
+import process from 'process'

-if (require.main === module) {
+if (process.argv[1] === fileURLToPath(import.meta.url)) {
   main();
 }

It's not hard to find lots (55K+) of examples following this pattern: https://github.com/search?l=&q=%22require.main+%3D%3D%3D+module%22+language%3AJavaScript&type=Code

tschaub commented 4 years ago

A shortcoming of the process.argv[1] === fileURLToPath(import.meta.url) workaround is that it doesn't work if someone runs the script without the .js extension.

For example, the workaround works when invoked like this:

# here the script can detect it was run directly
node script.js

But not when invoked like this:

# here the script can't detect it was run directly
node script

Update: see es-main.

jkrems commented 4 years ago

One random idea: what if node invoked an exported function main instead? If the entry point exports a function called main, it is invoked after the module graph is initialized. Maybe even passing argv..? That seems like a much more testable (and clean) pattern than top level conditional code.

ljharb commented 4 years ago

Ugh, we're not C++ or java, let's not do that :-/

jkrems commented 4 years ago

It’s not just Java/C++ that does it. I’m not sure if there’s a more complete argument than “languages I don’t like do something that looks similar”..?

devsnek commented 4 years ago

module === require.main is a fine pattern for commonjs, but esm is not commonjs. I don't think we need to feel forced to find a solution. It should be expected that migrating to a new system involves making changes that are more than a single line of code.

addaleax commented 4 years ago

@jkrems How about “It’s easier to make the very first steps with JS/Node.js this way”? I know that might seem like it’s not much in the big picture, but I do like this about JS.

bmeck commented 4 years ago

WASI is going to be taking the main() route (different from start); this might be something to discuss.

jkrems commented 4 years ago

Just to be clear: I wasn’t suggesting removing top level code. I was suggesting this as a way to mark some top level code as conditionally executed only when the file is running as the entrypoint. I hope that in most cases (especially for beginners) that’s not something people would have to worry about. :) Having separate files is almost always easier.

But yes, I can see why to some people a special condition + if block could be easier to explain than a “magical” function name. Just tried to understand if that’s what Jordan had in mind or if there are other arguments we should consider.

FWIW: the reason I suggested the main function was that in my experience the opposite has been true. The if condition seemed magical to people and having a clean function with the actual logic helped.

devsnek commented 4 years ago

@bmeck probably something to discuss on the esm proposal instead? We'd want all the different js environments that support wasi to do it uniformly.