esm: add import.meta.sync

[marco-ippolito]

idk why but I was trying to open the PR again my fork but it keeps opening to nodejs/node regardless...
Anyways here we are gathering some early feedback before we follow the whole wintertc process... etc
Hopefully this will alias https://github.com/tc39/proposal-import-sync if it ever gets accepted

[nodejs-github-bot]

Review requested:

• @nodejs/loaders
• @nodejs/vm

[aduh95]

To open the PR against your fork, you'd need to sync the main branch with upstream and then go to marco-ippolito/node@main...marco-ippolito:node:import-async

[marco-ippolito]

To open the PR against your fork, you'd need to sync the main branch with upstream and then go to marco-ippolito/node@main...marco-ippolito:node:import-async

Usually I just set the target branch to marco-ippolito/node 🤷 I think its because my fork main is not sync'ed with upstream main.
But anyways now its open

[ljharb]

import.meta is intended to have module-specific information, not to be a general purpose area for hosts to stick stuff on. This really feels like an abuse of import.meta.

[joyeecheung]

I think the naming should still emphasize that it's import, so import.meta.importSync would be better. The difference here is that when it encounter a package that has {"exports": { "require": "bar.js", "default": "foo.js" }}, it should always resolve foo.js (otherwise, this should probably be import.meta.require), and without import in the name it's not very clear whether it would skip bar.js - also to that end, while it might be too early to review the implementation just yet, mind that the kImportInRequiredESM used in createImportMetaSync may lead to an incorrect resolution and it should probably use kImportInImportedESM or add a kImportSyncInImportedESM to make sure the resolution is correct.

[ljharb]

I guess I'm also confused why this is better than making Module.createRequire more ergonomic - since require has the semantics that ES Modules aren't supposed to have already, and that this PR tries to add.

[marco-ippolito]

I guess I'm also confused why this is better than making Module.createRequire more ergonomic - since require has the semantics that ES Modules aren't supposed to have already, and that this PR tries to add.
#55730

[ljharb]

I remember that, #55730 (comment) as well - but if it's just "sync import" then you can't do either of those two use cases. Given top-level await, so you can await import(), what's the benefit of adding this? What use cases does it solve or improve?

[joyeecheung]

Another note: since it's intended to be an equivalent of import.sync, it should also take an import attribute, otherwise it would not be able to e.g. import JSON (that would also be another case where it'd differ from something like import.meta.require() and could use some clarification in the naming).

[marco-ippolito]

I remember that, #55730 (comment) as well - but if it's just "sync import" then you can't do either of those two use cases. Given top-level await, so you can await import(), what's the benefit of adding this? What use cases does it solve or improve?

Its clearly written in the documentation. Conditional synchronous imports.

Another note: since it's intended to be an equivalent of import.sync, it should also take an import attribute, otherwise it would not be able to e.g. import JSON.

I agree, will add it later if there is some consensus

[ljharb]

You can already do conditional synchronous imports with if (condition) { await import(…); }.

[marco-ippolito]

You can already do conditional synchronous imports with if (condition) { await import(…); }.

Thats async... 😄 You need to rewrite the function to be async (or tla)

[joyeecheung]

I think a use case that have come up before during the discussion of the import.meta.require is something like this:

function aSyncAPI(plugins) {
  for (const p of plugins) {
    const something = import.meta.sync(p);
    doSomething(something);
  }
  // do other things after the plugins are all initialized.
}

[ljharb]

Ah, you mean conditional synchronous imports not at the module level.

The standard way to do that will be using deferred imports, and then normal JavaScript (simply referencing the identifier would cause the import to happen immediately/sync) - in which case this PR would immediately be obsolete. Is it worth adding this now when any ecosystem adoption will imminently become a legacy problem?

[joyeecheung]

The standard way to do that will be using deferred imports

Did you mean something like import defer * as c from "plugin-a"? I think that would still require knowing which plugin to resolve statically, which wouldn't work for the example which exposes a synchronous API that take whatever plugins that users pass in?

[marco-ippolito]

I prefer import.meta.importSync too, the naming was just to shadow the stage 1 proposal import.sync
@guybedford

[joyeecheung]

Reading #55730 (comment) I remembered that being importSync this must always return the exact namespace; so compared to import.meta.require, this would not be able to fill the gap of "customizing the returned value in the case of a default export (so that converting to ESM is not a breaking change)", only the gap of the lack of synchronous, dynamic loading.

[joyeecheung]

Also, just food for thought (not proposing it as anything to be provided to the user land), if the ecosystem did not already universally accept that const foo = require('foo') is equivalent to import foo from 'foo', I'd probably provide an alternative API that gives const { promise, namespace } = importSync('foo'), so that the namespace is separated from the promise and the importer has greater control over the evaluation. That is the API I am adding internally to load built-in ESM in #60518 (in this case, the V8 tick processor actually uses TLA, but with this API I get to control when to await on that async evaluation, without having to go through the per-isolate import() callback and do an extra C++ -> JS roundtrip, and the names - not values - can be available before the evaluation is finished)

[Renegade334]

Did you mean something like import defer * as c from "plugin-a"? I think that would still require knowing which plugin to resolve statically, which wouldn't work for the example which exposes a synchronous API that take whatever plugins that users pass in?

Wouldn't this be covered by dynamic import.defer()?

[joyeecheung]

Wouldn't this be covered by dynamic import.defer()?

I think you'd be referring to import.sync() which is what this API tries to polyfill. The current semantics of import defer IIUC is that resolution and loading are eager - hence the specifier must be static - and only evaluation is lazy. If the specifier is dynamic and resolution is also "deferred", then it's just import.sync().

[guybedford]

we are gathering some early feedback before we follow the whole wintertc process... etc

WinterTC process or TC39 process? I think it's important to be clear about your approach here.

Hopefully this will alias https://github.com/tc39/proposal-import-sync if it ever gets accepted

Import sync has been accepted by TC39, and you are a champion of the proposal. The only limitation here is yourself!

[jasnell]

I'm a bit reluctant to do this this way now. I think it would be best to hold off a bit longer and see where the tc39 process goes with import.sync

[marco-ippolito]

@guybedford yeah I mean wintertc we are adding a need property in the import.meta namespace.
As per TC39 I need help with the writing the spec 😅

[guybedford]

@marco-ippolito is it your plan to do both import.meta.sync and import.sync then? Is that a good idea?

[marco-ippolito]

I guess if things go well with import.sync it becomes just an alias. I think it helps push the cause regardless

[guybedford]

@marco-ippolito I think you need to either stick with TC39 process or go with the WinterTC approach but not both.

[marco-ippolito]

@marco-ippolito it's a bit of a dirty game to play two committees. I think you need to show more integrity here.

What?💀
the only thing that needs to happen with wintertc is add it to
https://github.com/wintercg/import-meta-registry as per guidelines
I will ignore your comment as I find it disrespectful

[guybedford]

I've corrected my comment to (re-edited 11/15):

I think you need to either stick with TC39 process or go with the WinterTC approach but not both.

I stand by the comment though - this feels more like a political strategy than an honest approach to committee engagement and implementation by taking two appraoches at the same time, at the risk of a worse off result for the ecosystem. This is not ad hominem, this is a genuine ecosystem concern.

Returning to a a purely technical discussion (and in good faith as always), having both import.sync and import.meta.sync seems clearly unnecessary. Do you want to see import.meta.sync implemented on the web, or are you okay with this being Node.js / runtime specific?

[marco-ippolito]

It's a stage one proposal and for as much as I would like to see it happen, it's gonna take a long time especially since for the web it piggybacks other proposals. I don't see why we can't have it in the runtime.
I'm not gonna sit here getting disrespected from you talking about my moral intergrity, you can champion the tc39 proposal yourself I dont want to be part of it anymore.

[GeoffreyBooth]

A way to avoid these import.meta.x vs import.x debates entirely would be if it were somehow possible to pass the context of the current module into something being imported. So like if there was a way to make this code work:

import { importSync } from 'node:module'

importSync('./foo.js')
// where `./foo.js` resolves relative to this module,
// the same way it would resolve for `import()`

Or this:

import { require } from 'node:module'

require('./foo.cjs') // Look ma, no `const require = createRequire(import.meta.url)`

If it were somehow possible to create the importSync or require APIs from these examples, where they know the import.meta.url of the importing module when they’re run and can adapt accordingly, we could create APIs that are more convenient than any existing alternatives and wouldn’t need approvals from standards committees. Maybe these APIs are possible via C++ magic; I've tried to achieve them in pure JavaScript and couldn't find a way, short of unworkable hacks like inspecting the call stack of caught exceptions. If someone could figure out a way, though, it would unlock many useful APIs.

[guybedford]

I didn't plan for my last comment to be the end of the discussion here, so I'd like to first address the communication issues here, and then also give some further background into the cross-standard questions that may not be widely known, looking towards a hopeful path forward on the topic.

Firstly to just say, if it were not for Marco, amongst many other things we would simply not have TypeScript support in Node.js. This is a huge accomplishment, and his contributions to the project are hugely valued. I have nothing but respect for his work and always enjoy seeing his contributions and I've said this to him personally. The last thing I would want to do here is cause contributor frictions from insensitive comments.

To clarify my words - it is the specific approach and strategy to standards in this PR that I was questioning, and not Marco's character in any way.

On to the background - Marco originally posted #55730 last year. My stance on that discussion was the same as it is here - that it is my opinion that execution primitives should be part of TC39. This ensures standard behaviours between runtimes and avoids us losing all that we won in the migration from CommonJS to ESM in having a standard module system. We want import.sync to work for source phase imports, module declarations, module exprssions and WebAssembly modules and have clear semantics outlined through TC39 for these interactions.

Now I don't expect everyone to agree with me on the technical details, and Marco does not need to agree on this, that is not what this discussion was about.

This discussion was about the fact that Marco agreed to collaborate on import.sync at TC39, and said he was interested in doing this and joined as a co-champion, but then wanted to also take import.meta.sync to WinterTC without withdrawing the TC39 proposal. I assisted Marco in getting the proposal together and taking it to Stage 1. This was not something I was planning to do but did it to try and help the use cases he was caring about in a way we both agreed could work.

Having taken import.sync to Stage 1 with Marco as a co-champion, as a fellow TC39 member I was clear that it would take his efforts to help get through subsequent stages in spec text, presenting, tests, implementer discussions etc. No efforts were taken further.

I strongly believe import.sync is a great path forward for TC39 and the right level to solve this concern. It has my full support and support of other members of TC39, it just needs someone to do the work.

Marco has since asked to no longer be a co-champion of the proposal, so this proposal is now seeking co-champions. I'd like to use this opportunity to ask anyone interested to please consider coming forward to assist in collaborating on the import.sync spec if they care about this feature, TC39 member or not. I'd be happy to collaborate with anyone seeking to help and provide follow-up presentations at the committee. I'm also actively involved in various modules implementation work as part of my role as a TC39 delegate at Cloudflare for workers and we may be interested in the feature too. We can get this over the line if someone cares. I'd also still be happy to collaborate with Marco as well if he chooses to pick this up again anytime in future.

[jasnell]

I can help with championing... but would really like to ask @marco-ippolito to reconsider. It's definitely good work that deserves to move forward.

[marco-ippolito]

The issue here isn’t technical. I’m still open to collaborating, but I need to be clear that the way you addressed me wasn’t appropriate. As I mentioned in private, an acknowledgment of that would go a long way toward resetting the conversation so we can move forward productively.

[guybedford]

Sure, I can certainly publicly apologize for any insensitivity in the way I made my remark. To say "show some integrity" is indeed a very rough phrasing for what I was trying to convey, and I genuinely apologize for that and with sincerity for future collaboration.