Skip to content

Conversation

@marco-ippolito
Copy link

Refs: nodejs/node#55730

I will update documentation once it lands

@guybedford
Copy link

ES modules have an important analysis property over CommonJS modules - the ability to determine apart from eval all the static and dynamic imports in use.

This property is effectively invalidated by providing a import.meta.require as build tools now need to do full analysis of this expression, and unlike dynamic import() which cannot be assigned (var x = import doesn't work as it is syntactical), var meta = import.meta can be assigned and escape analysis resulting in a hybrid module system which invalidates this static analysis property of ES modules.

For WinterCG runtimes, having to support import.meta.require doesn't just require a build step, but a runtime support as well (due to analysis escapes as above), which requires an entire CommonJS implementation, effectively making CommonJS itself a WinterCG standard.

For these reasons, I don't feel this belongs in the WinterCG. I'm happy to discuss further or we could bring this to the WinterCG meeting as well.

@GeoffreyBooth
Copy link
Contributor

making CommonJS itself a WinterCG standard.

I think the purpose of the registry is just to claim keys so that if other runtimes choose to support them, they support them in the same way. It’s not a requirement that every runtime implement every import.meta key.

The arguments about static analyzability are persuasive, but since Node already has require = createRequire(import.meta.url), doesn’t Node already have this problem? At least for anyone using createRequire. Yes this API makes it easier, reducing the boilerplate to get a require from two lines to a few characters, and therefore people will use it more often; but since we already have non-statically analyzable code in ESM already, does it matter so much whether we have a little or a lot?

@guybedford
Copy link

While this repo is maintained as a namespace reservation system, it is being used to gain support for standards, and that is completely natural and we may need to adjust process to work with that reality as well. For example it may help to clearly indicate WinterCG standards versus platform-specific properties like in https://github.com/wintercg/import-meta-registry/pull/7/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R27). At the end of the day though, everything Node.js implements is effectively a standard, the question is really just if it's expected to work outside of Node.js as well.

I'd argue as a kind of intrinsic, import.meta has a higher bar to consider for cross-platform support than host-specific imports which are clearly node-prefixed (import 'node:module').

In terms of static analysis, createRequire by design offers no guarantees of static analysis / bundler support, so effectively works around the WinterCG platform concerns by naturally remaining Node.js-specific in its usage patterns, whereas import.meta.require may more easily become a standard beyond Node.js code since bundlers will then have to support it, and thus polluting the entire WinterCG ecosystem.

So yes I think it matters a lot to change these ergonomics.

I'm also happy to discuss use cases further - the desire for sync imports and the desire for sync asset loading. There are many ways to approach these problems and it would be good to work through those discussions as well without jumping to this solution specifically.

@ljharb
Copy link
Member

ljharb commented Nov 6, 2024

@guybedford dynamic imports take an expression, as does require, so both can be statically checked when they're static strings, and neither can be statically checked otherwise. In other words, CJS and ESM imports are precisely identically statically analyzable, as far as I can tell. Can you help me understand where I'm mistaken?

It is certainly true that import() can't be assigned to a function, but you can do const gimme = x => import(x); and create the same problems.

@guybedford
Copy link

As I mentioned in #9 (comment), the different property is that so long as there is no eval, import(...) can always be statically found in the source code - we know where in the execution graph a dynamic import might happen, and we can explicitly know the ... expression inside it. With import.meta.require you can have reassignment - var req = import.meta.require and then that req binding can escape analysis meaning all unknown bindings from an analysis perspective might be requires.

This is a major change in module analysis semantics that a module with no other imports may have a hidden runtime require function in it.

@ljharb
Copy link
Member

ljharb commented Nov 6, 2024

Thanks, I think I understand your explanation. I'm still not clear on when this actually causes problems in practice, though (since the specifier can't always be statically known regardless). Do you have a concrete use case that this would break, that you could elaborate on?

@guybedford
Copy link

Yes this is a different aspect of the analysis of a dynamic specifier expression - it's the guarantee not about knowing what the dynamic specifier expression is, but where the dynamic import expression itself is. That is, we lose being able to statically know all import() locations which can cause module loading.

@ljharb
Copy link
Member

ljharb commented Nov 6, 2024

Right - what's the impact of that loss, though? Or put another way, what's the benefit of having that knowledge?

@guybedford
Copy link

import() is a capability. Given an ES module to analyze you can draw conclusions about this capability:

  • If the module has no dynamic import: therefore, it only has access to the modules it statically imports.
  • If it has a dynamic import with a static expression string: therefore, it only has access to the modules known to be within the string variation space of that static expression string
  • If it has a dynamic import with a dynamic expression binding: it might dynamically import anything, but only so far as execution paths flow through that dynamic import, and when execution paths can be ruled out to not go through that dynamic import we can know that no other modules are accessed.

The importance of determining module access is because WinterCG runtimes want to ahead of time compile all JS code and do not want to have dynamic runtime code loading.

@ljharb
Copy link
Member

ljharb commented Nov 6, 2024

So, a file that has import() without a static string would fall into the third bucket - as would a file that has any import.meta.require usage that wasn't calling it with a static string? What's the difference between the two? (in both cases you can create a function that wraps them and pass that around, obscuring the actual callsite)

@guybedford
Copy link

guybedford commented Nov 6, 2024

With import.meta.require, it is possible to do things like const { require } = import.meta that lead to the require binding escaping call site analysis, thus ruling out comprehensive analysis, whereas import() can never escape call site analysis since it is syntactical.

@ljharb
Copy link
Member

ljharb commented Nov 6, 2024

I get that, but since i can make a function x => import(x), the callsite is what calls that function, not the location of the actual import, no?

@guybedford
Copy link

guybedford commented Nov 6, 2024

The final call site is the import(x) itself though - that represents the capability. Once you wrap import() in another function that fully exposes the input and output you end up with basically the same level of analysis properties as require(). That is, one can make import() analysis guarantees as weak as require() guarantees with a wrapper, but the default ergonomic usage of import() still provides stronger static analysis guarantees than require().

@ljharb
Copy link
Member

ljharb commented Nov 6, 2024

So if the nonzero possibility of user-weakening already exists, why does the existence of import.meta.require - an additional possibility of user-weakening via equally unergonomic means - change that calculus?

@guybedford
Copy link

guybedford commented Nov 6, 2024

Because even while analysis of import() can be weakened, import.meta.require would represent a new vector of weakening of the static analysis one can do for module import sites. You can't just tell users not to do const { require } = import.meta. So we lose what we gained with ESM to begin with in having a more static module system. And that's not something to throw away lightly just for some sync JSON loading, because someones too lazy to import the FS module.

@marco-ippolito
Copy link
Author

someones too lazy to import the FS module

This is meh, it's a significative dx improvement.
If the drawback is to weaken a mechanism that is already unreliable doesnt sound too bad.
Although not super relevant but afaik Bun is already using import.meta.require.

@aduh95
Copy link

aduh95 commented Nov 6, 2024

If the goal is to improve the DX of loading JSON files, we should make an API that does that instead of exposing require which is a much bigger thing IMO.

@marco-ippolito
Copy link
Author

marco-ippolito commented Nov 6, 2024

If the goal is to improve the DX of loading JSON files, we should make an API that does that instead of exposing require which is a much bigger thing IMO.

Obviously not only for JSON files.
We already have createRequire this is just an alias for whatever createRequire does. It improves the DX of createRequire

@guybedford
Copy link

I think we need to spend more time answering the question - why do we need to improve the DX of createRequire? And how are these not just the same arguments for an import.sync('./mod.js')?

@GeoffreyBooth
Copy link
Contributor

the same arguments for an import.sync('./mod.js')?

Is that possible?

@guybedford
Copy link

Is that possible?

I don't see why not - check if a module can be loaded sync, or it is in the registry cache, and execute it if necessary, throwing on TLA. Or just throw an "unable to execute sync" error.

@marco-ippolito hypothetically, if we had an import.sync would that solve your use case? I'm curious. For me that would mitigate my concern about the analysis weakening per the discussion above.

@marco-ippolito
Copy link
Author

marco-ippolito commented Nov 8, 2024

Im not familiar with import.sync proposal, can you point me to some doc? From the top of my head I would say yes, that would solve my use case

@guybedford
Copy link

There is no import.sync proposal, but the import defer proposal (https://github.com/tc39/proposal-defer-import-eval) has had to tackle and solve many of the semantic questions of import sync already.

So my question is are we just working around the non-existence of import.sync by wanting an import.meta.require or are there other things import.meta.require solves apart from the sync import capability?

If there really is interest in this from Node.js, then as a representative for Node.js and the OpenJS Foundation at TC39, I could present on an import.sync at the next TC39 meeting for discussion if this is something Node.js is interested in.

@GeoffreyBooth
Copy link
Contributor

are there other things import.meta.require solves apart from the sync import capability?

I think that's the main motivation. Technically the other thing that require provides in contrast to import is the ability to load a .node file but that's a very niche use case. An advantage of import.sync is that it's more obvious that it can import anything that import can, including both CJS and ESM. Many users might assume that require is only for CJS. I think import.sync would be more logical.

Wouldn't import.sync need to not just go through TC39 but also be added to V8?

@guybedford
Copy link

Wouldn't import.sync need to not just go through TC39 but also be added to V8?

Certainly it would, but when we get first-class bundles with module declarations and module expressions on the web there may even be web-first motivations to be had for it. Either way it could be worth a discussion at least especially if this is something that would solve a real problem for Node.js.

@marco-ippolito
Copy link
Author

I dont think I would want to get in the discussion for import.sync as its yet another way to import a file and it's in a stage 0 status (afaik). Users are already familiar with require, it's already available with createRequire, so its just an alias. It has very nice ergonomics. Would you it make you feel more confident adding an experimental warning to it?

@guybedford
Copy link

If the only reason for import.meta.require is due to a limitation in the ES module system that it does not have a sync import, then we should fix that limitation and not work around it. Taking the easy path here pollutes the ecosystem forever.

Therefore I am completely against any WinterCG endorsement, since if WinterCG and Node.js supports it then it would become a permanent JS standard.

@guybedford
Copy link

Certainly if there were no standards or alternatives paths I could empathize with the use case - but I'm completely offering to actively help with standardization here, and could present as soon as the next TC39 meeting.

@GeoffreyBooth
Copy link
Contributor

I feel like we shouldn't ship import.meta.require just because it's faster to standardize than import.sync. The latter feels like a better API.

This does raise a meta point that there's a risk of TC39 being bypassed if its process is just too cumbersome.

@marco-ippolito
Copy link
Author

marco-ippolito commented Nov 11, 2024

If import.sync proposal lands I'm happy but this is a different API that behaves differently. I'm pretty sure import.sync will behave like import.
import.meta.require behaves like require, let's not mix the two things.

  • its an alias
  • has good ergonomics
  • its already used by another runtime
  • its something that users are already familiar with

Also afaik WinterCG endorsement doesnt mean that other runtimes HAVE to implement the api, but if they decide to, it should behave in the same way.

Node already has the createRequiee api, I just want to make it better, other runtimes already have require for node compatibility, so whats the issue?

@GeoffreyBooth
Copy link
Contributor

I'm pretty sure import.sync will behave like import.
import.meta.require behaves like require, let's not mix the two things.

What's the difference? As far as I can tell, import.sync would require a bit more verbosity to import JSON, and wouldn't support .node files. But other than that aren't they equivalent?

@marco-ippolito
Copy link
Author

marco-ippolito commented Nov 11, 2024

I'm pretty sure import.sync will behave like import.
import.meta.require behaves like require, let's not mix the two things.

What's the difference? As far as I can tell, import.sync would require a bit more verbosity to import JSON, and wouldn't support .node files. But other than that aren't they equivalent?

Extension-less for example? I don't think they are equivalent at all.
Again if import.sync lands that's great, but its not related to this.
Consider the PR in Node is only blocked by this and has (afaict) consensus to land.

@aduh95
Copy link

aduh95 commented Nov 11, 2024

  • has good ergonomics

That’s subjective

  • its something that users are already familiar with

CJS users, sure. But IMO, we should not take familiarity with CJS for granted, there are new JS users everyday, and it doesn’t seem unlikely they would familiarize themselves only with ESM, so having a CJS API on all ES modules only muddies the water on that regard.

Consider the PR in Node is only blocked by this and has (afaict) consensus to land.

Consider the PR on nodejs/node also blocked by me for the above reason. I’m fine with this PR though.

@marco-ippolito
Copy link
Author

marco-ippolito commented Nov 11, 2024

Ok closing both PRs since there is no consensus.
Disappointed to the fact that the reason against it are:

having a CJS API on all ES modules only muddies the water on that regard

We already have that, its called createRequire, this is just an alias.

the only reason for import.meta.require is due to a limitation in the ES module system that it does not have a sync import, then we should fix that limitation and not work around it

true, but again we already fixed this limitation in Node and at least one other runtime, so that would be great to have in spec but I dont see why we shouldn't improve current DX

I feel like the conversation is not going anywhere, and the reasons against are not more than the pros imho, but I don't have the energy to continue the discussion after I have explained my points.

@GeoffreyBooth
Copy link
Contributor

at least one other runtime

At least as of today, import.meta.require is undocumented in Bun; before we add it to WinterCG, we should confirm that whatever we’re specifying actually matches the signature and semantics of what Bun provides.

@guybedford
Copy link

I've created https://github.com/guybedford/proposal-import-sync to further investigate the potential for a import.sync for Node.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants