Importing a module that is in a bundle, that gets System.set causes an assertion error

[matthewp]

bundle-a

System.set("a", System.newModule({}));

main

System.import("a");

This assertion will fail: https://github.com/ModuleLoader/es6-module-loader/blob/dff97c6b9783562b433a13c6806809e5b8c6bbda/src/loader.js#L564

[guybedford]

Yes - you cannot set over the existing module load. You have to load the bundle first. An in-flight import to a, can't be set while it is still in flight. This is a natural limitation of the pipeline, and another reason why using System.set in bundles is not encouraged.

[matthewp]

Is this going to be fixed in the new spec? There's no reason why the spec shouldn't support this. A call to System.set/install should cause the in-flight import to be resolved.

[guybedford]

Not that I know of.

[matthewp]

Ok, i'll bring up the issue there then. By the way, you said:

This is a natural limitation of the pipeline, and another reason why using System.set in bundles is not encouraged.

Where would you put them then? Just another script before importing your main?

[guybedford]

I'd really suggest using the System.register output, supporting circular references and avoiding this issue at the same time. Or write your own extension implementation that bypasses System.set and allows the instantiate function to pick up from the bundle like System.register.

[matthewp]

Sure, but doesn't this bug makes System.set/install effectively unusable because you cannot know if an in-flight import is out there for the module you are installing?

[matthewp]

This is enough to cause the bug:

  var fetch = System.fetch;
  System.fetch = function(load){
    System.set("main", System.newModule({}))

    return fetch.call(this, load);
  };

  System.import("main");

I don't know how i can ever safely use System.set since I do not know if there is an importPromise for the module already. I'll move the discussion to the loader repo.

[matthewp]

@guybedford Might this be fixable in user code? I hate that it's necessary but I wonder if there was a instantiate hook that checked loader.has, got the set value, deleted the module, then return an execute that returned that value... just thinking out loud.

[matthewp]

nm, I'll just stop using system.set for this purpose and hopefully convince whatwg/loader to fix this.

[matthewp]

I posted whatwg/loader#48

I'm not sure what the intended use of loader.cancel is, but it could fix this.

[guybedford]

Did you manage to work around this ok? Don't know much about loader.cancel either, but that sounds like a reasonable approach if it behaves like you suggest and is synchronous which it quite possibly could be.

[matthewp]

I can work around it for this one specific use case, but there are other use cases for System.set/install (dynamically creating modules) that are inherently unsafe. If you don't mind keeping the issue open for another week or so I'd like to circle back and see if there's a possible code solution.

[guybedford]

Yes best is to ensure System.set runs either before System.import for that module could ever run, or after the module is definitely set in the registry. Anything else will be unpredictable as you say. But the two use cases covered in those edges are testing / mocking and hot-reloading, which I'd argue are the only use cases that make sense for System.set.

[matthewp]

I don't think we can honestly declare all possible use-cases for an API. A year, year and a half ago, we wouldn't have been talking about hot-reloading at all, so I imagine new use cases will emerge in the future. Fundamentally the ability to dynamically create modules in code is a powerful and I think valuable feature of the spec. So I'm hoping .cancel can solve this issue, otherwise that the spec authors listen and address the issue in some way.

[matthewp]

Closing as this is a spec bug :(