nodejs / modules

Node.js Modules Team
MIT License
413 stars 42 forks source link

Spec: Would `!!import.meta ? 'module' : 'not'` be a reasonable ask? #395

Open SMotaal opened 4 years ago

SMotaal commented 4 years ago

I'm think we all must have considered this at some point, but cannot recall.

Q: Is there a safe way for code to test if it is module or not (ie not early/parse error)?

This code would target runtimes supporting import(…) globally, irrespective of semantics, and so my thinking was a clean nice-to-have here could be !!import.meta ? 'module' : 'not' which should not be too much to ask for… thoughts?


Possibly Related Discussions

ljharb commented 4 years ago

If code wants to assert its parse goal, it can do export {} to force ESM or with ({}) {} to force Script.

You can also wrap either of those in Function and a try/catch to detect it.

SMotaal commented 4 years ago

You can also wrap either of those in Function and a try/catch to detect it.

This one is more on point, but I hit this issue in node:

// sometimesModule.js — {package: type: 'module'}
console.log(
  (function() {
    try {
      import.meta.url;
      return 'module';
    } catch (e) {
      return 'not';
    }
  })(),
);
> node sometimesModule.js

SyntaxError: Cannot use 'import.meta' outside a module
    at Module._compile (internal/modules/cjs/loader.js:872:18)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:947:10)
    at Module.load (internal/modules/cjs/loader.js:790:32)
    at Function.Module._load (internal/modules/cjs/loader.js:703:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:999:10)
    at internal/main/run_main_module.js:17:11
> node --experimental-modules sometimesModule.js

(node:37127) ExperimentalWarning: The ESM module loader is experimental.
module

So what I was getting at is sometimesModule.js would be an entrypoint that could be portable everywhere and in case it is node, it will either require or import('module').createRequire(…).

If code wants to assert its parse goal, it can do export {} to force ESM or with ({}) {} to force Script.

And so, this is a separate topic, but thanks.

ljharb commented 4 years ago

(I crossed out that part of my comment since Function always evaluates as a sloppy Script)

Why would you want a file to be both Module and Script? Can you help me understand the use case?

bmeck commented 4 years ago

At the top level this differs between Script and Module.

// maybe "use strict" here if you wanna have a Strict Script
let isModule = this === void 0;
// ... rest of file ...
bmeck commented 4 years ago

@SMotaal are you still wanting to pursue adding import.meta to Script or is that enough?

SMotaal commented 4 years ago

Why would you want a file to be both Module and Script? Can you help me understand the use case?

@ljharb Not a good way for me to be able to reframe problems (ie can you humor me anyway please), but say this is a workshop for newcomers, where they use the sometimesModule.js to bootstrap.

I'm inclined to think that for someone less experienced, an intuitive expectation would be to know for certain that they are not in a module.

As @bmeck points out:

At the top level this differs between Script and Module.

And certainly, this is not as intuitive, but also worth being aware of. It still can get messy to explain for such audience, though

Because there are other ways for such a module to evaluate with this being undefined that can merely be due to a bug in a some loader on some platform:

const evaluate = (1, eval)(`
  function() {
    "use strict";
    return eval(arguments[0]);
  }
`);

It is fair imho to say that semantics of this being undefined and the code being evaluate itself is module or not are rough and hacky to relate to for at least a few folks.

Therefore: The argument I am trying to consider making is that semantically speaking, import.meta is an object in module, otherwise not — and that could just as well be undefined.


(I crossed out that part of my comment since Function always evaluates as a sloppy Script)

Yup, was mid way trying to clean up the code I did towards it and saw it as I replied… thanks though!

ljharb commented 4 years ago

It seems to me that newcomers should understand that the two parse goals are different, and which one they’re using, before writing a line of code - just like they’d need to understand which language they’re writing beforehand.

SMotaal commented 4 years ago

@ljharb I appreciate there being different views on this, and appreciate others appreciating that when it comes to whohow to go about improving opportunities for newcomers learning, it is fair to assume that there will be different schools of thought, all worth appreciating.

Fair?

SMotaal commented 4 years ago

@SMotaal are you still wanting to pursue adding import.meta to Script or is that enough?

@bmeck Yes please — sorry, was midway in posting :)

SMotaal commented 4 years ago

@ljharb if we just considered how this plays out if it was introduced, where do you see !!import.meta causing runtime ambiguities or concerns?

devsnek commented 4 years ago

@SMotaal showing people the differences is good, but there's no reason to actually rely on these checks in code. Maybe if you're doing some advanced bundling or something, although even then it's iffy.

Some web people wanted import.meta in scripts (as an object) and I'm more interested in pursuing that.

ljharb commented 4 years ago

For one, the proposal for it explicitly and intentionally disallows it to be parsed in Scripts.

SMotaal commented 4 years ago

Some web people wanted import.meta in scripts (as an object) and I'm more interested in pursuing that.

@devsnek certainly… this is the path I was thinking down the road btw.

Something like (but not quite) !!import.meta && (!('type' in import.meta) || import.meta.type === 'module') ? … but I was thinking that would be in progression and open to variation.

So, first we want import.meta to not throw to pan out (the least amount of objections imho being just undefined at first), so this expression then would not be a parse error.

Workable?


For one, the proposal for it explicitly and intentionally disallows it to be parsed in Scripts.

@ljharb Can we know that the intent was to prevent this or could this just have been the leftover from the pre import() revisions?

devsnek commented 4 years ago

just to be clear, I'm against import.meta.type. The objects should be identical to the ones in modules.

SMotaal commented 4 years ago

just to be clear, I'm against import.meta.type. The objects should be identical to the ones in modules.

@devsnek I'm willing to leave this in for the right time, but would it be reasonable to consider letting it afford more meta about its source, irrespective of bikeshedding, fields… etc.?

SMotaal commented 4 years ago

Side question to help me appreciate things better please… I hope it is fair to ask, is there a sense of reluctance with wanting to block sometimesModule.js from ever panning out?

devsnek commented 4 years ago

sometimesModule.js shouldn't be possible. If it is possible, we have a bug.

ljharb commented 4 years ago

@SMotaal the intent is explicit: https://github.com/tc39/proposal-import-meta#proposed-solution

SMotaal commented 4 years ago

sometimesModule.js shouldn't be possible. If it is possible, we have a bug.

@devsnek It would be really helpful to help others understand why it should though? At least I disagree with this, but if there is more to it beyond preference I am more than happy to revise my position. Please help me appreciate your rationale for it.


@SMotaal the intent is explicit: https://github.com/tc39/proposal-import-meta#proposed-solution

@ljharb Assuming you might be referring to this bit (please be more explicit otherwise)…

and should not be repurposed for information about the currently-running classic script.

If so, that is natural in the progression of spec things… as in proposed things must not be watery, they must be explicit (and that is not the same as contention for what is being asked), but later things are subject to elicit such things to be changed, right?

devsnek commented 4 years ago

@smotaal ambiguity breeds confusion. This is one of the reasons I'm not a fan of type: module. Changing the location of a file on disk really shouldn't change how it's interpreted by node.

SMotaal commented 4 years ago

@SMotaal ambiguity breeds confusion. This is one of the reasons I'm not a fan of type: module. Changing the location of a file on disk really shouldn't change how it's interpreted by node.

@devsnek I am not talking about just node but even node can have more than one way of looking at a file for many reasons — ie same file but not the single instance of some node build — right?

devsnek commented 4 years ago

Sure different versions of node could do that, but they could also break your detection system. I don't think that's a path worth going down.

SMotaal commented 4 years ago

I am still feeling I am missing context which I really am curious about and hope you can appreciate me wanting to step back from those tangents and may be try and approach things differently…

It helps to directly restate the ask here… import.meta not being a parse error being an opinion of equal merit and one held by those using the language and hitting this to want to explore things more closely, as in purely about the semantics of ECMAScript, not specifics of some implementation, including our own work-in-progress, or node in general.

Two views on a matter here is okay with me, but if it is not merely refuting my view because it is opposite to your own, it might help to reconsider things from the other perspective, not consider it being wrong the foregone conclusion (sorry, this is just how it feels from my where I'm sitting) to the discussion, and that would help make this feel like a fair space for debating with good constructive arguments.

Conclusion: This clearly is not wrong from my perspective here, based on this discussion so far, just different and I have not picked up on anything that leads to this breaking semantics (aside from not throwing I mean).

jkrems commented 4 years ago

If all you want is a file that could be either, just uses import(), maybe just:

'use strict';

const isModule = typeof require === 'undefined';

Not sure why it would need import.meta - the above seems simpler than the import.meta check.

SMotaal commented 4 years ago
typeof require

here is fair for node specifically, not generally though — as in sometimesModule.js cannot dictate which runtime and loading approach is used, and just as loading can mean cjs or esm in node, it can mean more than just esm elsewhere because loading existed there too prior to esm times (ie not always just typeof require).

@jkrems fair?

ljharb commented 4 years ago

That's where this becomes the better choice.

SMotaal commented 4 years ago

Yes… but also above.

ljharb commented 4 years ago

If this doesn't work due to some loader, then the loader is broken - it's mandated by spec.

jkrems commented 4 years ago

here is fair for node specifically

Right, but what exactly do you want to tell apart then? If it's not about node specifically, there's way more than CJS (which isn't an ECMA script) vs. ESM. There's also script, multiple kinds of closure-compiler modules, etc.. Can you explain your use case?

SMotaal commented 4 years ago

I'll be checking back on this later, feels cyclic at this point, sorry, and really appreciate everyone's input.

@jkrems — sorry I am out of breath 2 hours later and realize you only just joined.

It seems that it is a fair discussion to keep open.

devsnek commented 4 years ago

@smotaal I'm not entirely sure what you're asking but here's what I've been saying: needing a check for whether you're in a script or module is nonsensical to me, so pursuing features for that purpose is also nonsensical to me.

As an example: the globalThis polyfill doesn't care about whether it's in a module or not, even though it has a this === undefined check. It cares about ways of finding the global this value.

GeoffreyBooth commented 4 years ago

This isn’t nonsensical at all. It dovetails with https://github.com/nodejs/modules/issues/389, where tools need to know whether they’re working in a CommonJS or ESM context and do stuff like import('./cjs-file.js') or import('./esm-file.js') as appropriate. There are many use cases for when an ambiguous file might want to know if it’s been loaded as CommonJS or ESM, or as Script or Module in a browser. And authors like @SMotaal in this case might want to create an intentionally ambiguous file in order to be a universal entry point that can then load a certain build as appropriate.

It’s not too different from the environment detection you see at the top of libraries like jQuery:

( function( global, factory ) {

    "use strict";

    if ( typeof module === "object" && typeof module.exports === "object" ) {

        // For CommonJS and CommonJS-like environments where a proper `window`
        // is present, execute the factory and get jQuery.
        // For environments that do not have a `window` with a `document`
        // (such as Node.js), expose a factory as module.exports.
        // This accentuates the need for the creation of a real `window`.
        // e.g. var jQuery = require("jquery")(window);
        // See ticket #14549 for more info.
        module.exports = global.document ?
            factory( global, true ) :
            function( w ) {
                if ( !w.document ) {
                    throw new Error( "jQuery requires a window with a document" );
                }
                return factory( w );
            };
    } else {
        factory( global );
    }

// Pass this if window is not defined yet
} )( typeof window !== "undefined" ? window : this, function( window, noGlobal ) {

There’s a name for this, I forget what it is. But it’s the same idea.

I would think that what @SMotaal wants is a variation of the above snippet, that doesn’t just detect between Node CommonJS and browsers but also Node ESM (and maybe between browser Script vs browser Module). Then it could do stuff like import('./jquery-modern-browsers.js') vs import('./jquery-legacy-browsers.js'), import('./node-commonjs.js') vs import('./node-esm.js'), etc.

devsnek commented 4 years ago

@GeoffreyBooth if import() is available it can be used unconditionally, that's why its available in all es parse goals.

the jquery example there is also dynamically choosing how to export, not import. There is no dynamic export anyway so detecting won't help you with that.

GeoffreyBooth commented 4 years ago

@GeoffreyBooth if import() is available it can be used unconditionally, that’s why its available in all es parse goals.

Yes, that’s exactly the point, that’s how it can exist in an intentionally ambiguous entry point.

the jquery example there is also dynamically choosing how to export, not import. There is no dynamic export anyway so detecting won’t help you with that.

Not sure what your point is. Lots of code doesn’t export, like the main code for an app. “if modern browser, import modern build, else import legacy transpiled build.” This use case happens all the time.

If what you had in mind is how to publish an NPM package that can be imported or required, yes, that’s impossible, because you can’t dynamically choose between module.exports or export. But that’s not what @SMotaal was asking here.

devsnek commented 4 years ago

i'm trying to figure out why someone needs to know if they're in a script or a module.

console.log(inScript ? 'script' : 'module') is a cute program but has little real-world relevancy. If you're looking for this, inScript is just a proxy to the real check, this === undefined, so inScript doesn't help you there. If you're looking for import(), its availability isn't dependent on inScript, so inScript doesn't help you there.

GeoffreyBooth commented 4 years ago

i’m trying to figure out why someone needs to know if they’re in a script or a module.

I’ve already given you a use case: a library for use in browsers that wants to be able to used as either Module or Script. Say you’re writing the Bootstrap JavaScript, to be hosted on CDN. You could tell users “use this URL with script type=module and this other URL with regular script” or you could provide a URL that works in both. The ambiguous script at that URL would be like this (psuedocode):

if (moduleEnvironment)
  import('./module.js');
else
  import('./script.js');

So a site like https://www.bootstrapcdn.com/ would only need to publish one URL, not two.

devsnek commented 4 years ago

@GeoffreyBooth if dynamic import is available, static import is available. That code makes no sense to me. You could replace the entire thing with import('./module.js');.

Also yes, you should use <script type=module> combined with <script nomodule>. I wish that wasn't the case but there isn't really anything we here in this issue can do about that.

MylesBorins commented 4 years ago

Dynamic import is available in the script goal, where static import is not available.

On Fri, Oct 4, 2019 at 1:36 PM Gus Caplan notifications@github.com wrote:

@GeoffreyBooth https://github.com/GeoffreyBooth if dynamic import is available, static import is available. That code makes no sense to me.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/modules/issues/395?email_source=notifications&email_token=AADZYV5TNJXD6AVAOWWRRHLQM55K7A5CNFSM4I5QNNLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAML4FI#issuecomment-538492437, or mute the thread https://github.com/notifications/unsubscribe-auth/AADZYV5DMK7YQZXZESIOHFDQM55K7ANCNFSM4I5QNNLA .

devsnek commented 4 years ago

@MylesBorins i edited my comment to clarify what i meant, apologies for the confusion.

bmeck commented 4 years ago

It seems the desire is still just to detect the current goal regarding any proposal, which we have a workflow that allows it. For detecting node there are existing workflows. I'm not seeing anything that needs a new feature explicitly.

SMotaal commented 4 years ago

@bmeck can you point to that (would really be help if there is one missed in this thread), but please note the above considerations — ie this===undefined and typeof require!=='function' being only workarounds that work under the right conditions — ie specific to certain implementation assumptions, and not explicit semantically speaking.

A language spec should not relate to a "by the chance of it there was…" kind of assertions that are not part of its own text, but rather explicit assertions that people can trust to author code around — or not in this case — because when there is no contract or guarantee that such implementation-specifics not spec being tested will only ever mean the implied thing to be asserted.

I really hope this is not considered a faulty premise to want us to work from and that others at least find it a little disturbing that we are repeating patterns that already showed us how future problems can be avoided.

bmeck commented 4 years ago

@SMotaal I'm not sure import.meta being undefined is any less esoteric in my mind. this === undefined is a contract guaranteed by the spec which should work in all conditions. The language itself cannot provide a guarantee about host provided APIs like require, that is the realm of feature detection; I'm not sure what is being proposed to make a contract there.

SMotaal commented 4 years ago

@SMotaal I'm not sure import.meta being undefined is any less esoteric in my mind. this === undefined is a contract guaranteed by the spec which should work in all conditions.

@bmeck please note above

The language itself cannot provide a guarantee about host provided APIs like require, that is the realm of feature detection; I'm not sure what is being proposed to make a contract there.

Also, I am not hard set on it being import.meta===undefined but rather any similar approximation for something that is more readily intuitive enmasse — that does not come with circumstantial trimmings like needing to guard for require because this is not a spec thing in itself — this being a premise in itself for wanting this detection without require being the test.

bmeck commented 4 years ago

@bmeck please note above

I've seen that; as I noted, it should be done at the top level. I would assume import.meta to be undefined inside of eval inside of Module source text as well. I don't see a uniform solution.

Also, I am not hard set on it being import.meta===undefined but rather any similar approximation for something that is more readily intuitive enmasse — that does not come with circumstantial trimmings like needing to guard for require because this is not a spec thing in itself — this being a premise in itself for wanting this detection without require being the test.

I don't understand this.

ljharb commented 4 years ago

If there can be a bug about top level this, there can be one with import.meta. It’s not worth speculating about possible bugs.

Top level this is a reliable means of differentiating Script and Module, per spec, in every engine or context I’m aware of. The possibility of bugs in that mechanic doesn’t alter reliability.

devsnek commented 4 years ago

fwiw, eval('import.meta') is always a syntax error.

SMotaal commented 4 years ago

I've seen that; as I noted, it should be done at the top level.

@bmeck It is fair to state that the intent here is for the code to know, "Top level" being an assumption that cannot be verified by the code itself here leaves the gap. The bigger gap here though is going from this===undefined to deducing module/not is quite a leap, my inclination towards import.… here would be to somewhat shorten this (cognitive) distance with what it is already on hand to work with today.

devsnek commented 4 years ago

I'm still really confused about what we're actually trying to enable here

ljharb commented 4 years ago

Code that is wrapped also can’t know if it’s transformed - which means import.meta, or any syntax, would be unreliable anyways.

SMotaal commented 4 years ago

fwiw, eval('import.meta') is always a syntax error.

@devsnek it does help, I somehow forgot to explicitly note it above… thanks.

Just to reiterate the thought at this point, pursuing first-class detection by code itself, with import.… merely being a more intuitive place to work with than this… and more so than the alternatives making implementation-specific stipulations (like require being or not anything).