tc39 / proposal-shadowrealm

ECMAScript Proposal, specs, and reference implementation for Realms
https://tc39.es/proposal-shadowrealm/
1.41k stars 67 forks source link

importValue: How to import a module with no exports? #364

Open Jack-Works opened 2 years ago

Jack-Works commented 2 years ago

According to ShadowRealm.prototype.importValue, missing exportName is a TypeError, and inside ShadowRealmImportValue, ExportGetter step 6 says, 6. If hasOwn is false, throw a TypeError exception.

This means I cannot import a module into the ShadowRealm with no exports. (Maybe I just want to let that script run in the ShadowRealm, I don't want any value returned.)

mhofman commented 2 years ago

I think this is related to https://github.com/tc39/proposal-shadowrealm/issues/350#issuecomment-1061918944 Throwing in this case allows for future behavior to be added.

ShadowRealm is a low level API. You likely won't be able to import most modules as-is with the current proposal, but instead have to use a "loader" module. What prevents you from doing that in this case? A dummy module with a primitive const export that has a side effect of importing your target module and optionally do any init? (IMO side effect execution on import is an anti-pattern and rarely warranted)

nicolo-ribaudo commented 2 years ago

Does importing it like this work?

myShadowRealm.evaluate(`await import("./module-with-no-exports.js")`);
mhofman commented 2 years ago

You can't do "top-level-await" in an eval. You could however manually wrap the promise:

await new Promise((resolve, reject) => {
  myShadowRealm.evaluate(`(callback) => {
    import("./module-with-no-exports.js").then(
      () => { callback(null); }, 
      (err) => { callback(String(err)); },
    );
  })((err) => {
    if (err != null) {
      reject(err);
    } else {
      resolve();
    }
 });
});
jdalton commented 2 years ago

The restriction on having an export caught me by surprise. I would have assumed this to be similar to APIs like Worker's importScripts. We had a question about how to import code into a realm when unsafe-eval is restricted via CSP. I would have thought importValue could do as an alternative, but with this restriction things get more tricky.

The current spec editors note for importValue states:

EDITOR'S NOTE Extensible web: This is equivalent to dynamic import without having to evaluate a script source, which might not be available (e.g.: when CSP is blocking source evaluation).

This restriction is counter to that note. The name of importValue does suggest a value. I lack context on why it was scoped as such though. If there's workarounds then the restriction should be brought into question as it appears to create artificial barriers for users.

ljharb commented 2 years ago

I agree; this seems like a strange restriction.

leobalter commented 2 years ago

@jdalton I believe that goes back to @mhofman's comment.

When importValue was initially designed, the intention was to not assume how to import or reproduce the module namespace object. With that in mind, the idea was requiring the second argument to always refer to a given name.

By the time, an explicit undefined argument in the name position ended up being cast as its string representation, we improved that.

We could say the explicit undefined argument refers to a no-name resolution, but that seems weird if we compare it to import(specifier) or a single parameter version of importValue.

I want to eventually expand importValue to cast a static serialization of the module namespace object if only one parameter is given, which also assumes the second parameter is not defined (or implicitly undefined).

Keep in mind importValue is ergonomic to a dynamic import, this is different from importScripts.

I'd be happy if we can also consider a ShadowRealm.prototype.importScripts but I wonder how is that specified only in ECMAScript.

mhofman commented 2 years ago

Technically without requiring eval you could have a trampoline:

loader.js:

export const load = (fullSpecifier, callback) => {
  import(fullSpecifier).then(
      () => { callback(null); }, 
      (err) => { callback(String(err)); },
    );
  });
};

main.js:

const load = await myShadowRealm.importValue('loader.js', 'load')
  .then((rawLoad) => (specifier) => new Promise((resolve, reject) => {
    const fullSpecifier = new URL(specifier, import.meta.url).href;
    rawLoad(fullSpecifier, (err) => {
      if (err != null) {
        reject(err);
      } else {
        resolve();
      }
   });
  });

await load('./module-with-no-exports.js');

I understand this is not ergonomic, but the only limitation is to be able to load the trampoline, which wouldn't be evaluated.

caridy commented 2 years ago

Maybe we should pull the trigger for "Wrapped Module Namespace Exotic Objects" instead. That will allow you to just do realm.import('foo') which will return a promise on the incubator realm that will give you access to things from the underlaying namespace object but following the wrapping semantics.

littledan commented 2 years ago

I am confused by this thread. importValue is a specialized API which only works for modules which were written with it in mind--there is more that you need to get right besides having an export. As others have mentioned, it is possible to achieve this behavior with either a wrapping module or wrapping code in eval. I would rather hold off on adding this sort of convenience until we have a broader story for how modules which weren't written with ShadowRealm in mind should be usable (e.g. a built-in membrane framework).

ljharb commented 2 years ago

If that's the case, then it seems like a subpar API - modules shouldn't have to know about ShadowRealms to be usable inside one.

mhofman commented 2 years ago

modules shouldn't have to know about ShadowRealms to be usable inside one.

They don't need to know. However the user of the ShadowRealm API should know that their entrypoint module needs to be compatible with the ShadowRealm API. None of the other modules that entrypoint references need to have that constraint.

ljharb commented 2 years ago

@mhofman good clarification, but that still seems problematic to me - i should be able to use any module with a shadowrealm, just like i can use any module with import().

mhofman commented 2 years ago

i should be able to use any module with a shadowrealm, just like i can use any module with import().

I think we'll just need to disagree here. shadowRealm.importValue() is not equivalent to import() and I don't think it needs to be,

littledan commented 2 years ago

We had a pretty extensive discussion about the importValue API in GitHub and TC39 plenary, including @ljharb --it's always been an API for importing a module built for it, just like passing functions over the ShadowRealm boundary is also for functions built for it (in the same way). I'm surprised by this new proposed requirement on the API shape. It seems like the kind of thing that should've been raised before Stage 3; I'd rather not make big changes on the shape of the API now (and I have no idea what these changes would look like).

ljharb commented 2 years ago

I'm confused at your phrasing - I don't think anyone proposed a requirement, nor am I suggesting changes. New information can be recognized at any stage, and I hadn't thought about it in these terms prior.

All normative changes are "supposed" to be made before stage 3, but this proposal has made some and is considering others, and other proposals have made many. The process isn't perfect, as we all know.

littledan commented 2 years ago

What's the new information here?

ljharb commented 2 years ago

I newly realized that this means that the module consumer has to know details about the module's authoring to be able to consume it inside a ShadowRealm. Philosophically, I don't think module consumers should ever have to know anything about a module except what kind of values it exports, under which bindings.

leobalter commented 2 years ago

I plan to discuss this in July so we can dedicate time for this topic along the other things we are already discussing this week at TC39.

I'd be very happy to have an import() equivalent but this brings a lot of design questions. I believe the resolved value should be a simple/ordinary clone of the given module namespace object that is non-dynamic.

We are aware of expressed objections for adopting Structured Serialization across ShadowRealms. Those are still current. I'm also predicting any sort of cloning for the module ns object will emerge the requirement from other delegates to adopt Structured Serialization, so we should expect blockers on both ways.

The current importValue might not be as powerful compared to a import method, but it resolves our requirements for now.

Perhaps we can understand that ShadowRealm.prototype.import can be part of the commitment from the champions of this proposal, but I hope it comes as a separate proposal in which I'm happy to start and present in the next meeting. My goal is to not block the current implementation, as this will have a severe impact on us and the web.