LoadModule doesn't support GetOrCreateLoad

[guybedford]

The exact same issue in https://github.com/jorendorff/js-loaders/issues/59 applies to the import function as well.

Note this means we get a TypeError from:

  System.import('jquery', function() {
  });

  System.import('jquery', function() {
  });
  // -> TypeError from AsyncStartLoadPartwayThrough, name is in loader.loads

[guybedford]

Surely RequestLoad should be used instead of LoadModule in all of these cases?

[guybedford]

Note this is a very critical issue to having a functional loader.

[guybedford]

The fix I'm using temporarily to get around this is to store the load record promise in the Loader:

    var importPromises = {};
    Loader.prototype = {
      define: function(name, source, options) {
        if (importPromises[name])
          throw new TypeError('Module is already loading.');
        importPromises[name] = Promise(asyncStartLoadPartwayThrough(this, name, 'translate', options && options.meta || {}, options && options.address, source));
        return importPromises[name].then(function() { delete importPromises[name]; });
      },
      load: function(request, options) {
        if (importPromises[name])
          return importPromises[name];
        importPromises[name] = loadModule(this, request, options);
        return importPromises[name].then(function() { delete importPromises[name]; })
      },
      import: function(name, options) {
        if (this._modules[name])
          return Promise.resolve(this._modules[name]);
        return (importPromises[name] || (importPromises[name] = loadModule(this, name, options)))
          .then(function(load) {
            delete importPromises[name];
            return evaluateLoadedModule(this, load);
          });
      },
      ...

[guybedford]

I understand that there is a conflict here that different options.source or other arguments may be provided, but the first import should win, and that shouldn't need to stop the second import from working.

[johnjbarton]

@guybedford I gather that it is your opinion that Loader.import(name) should succeed if 'name' already exists and, if ''name' is in process, then the second import should share the fate of the first. In other words duplicate import() calls or import() calls that duplicate import declarations are not an error.

[guybedford]

@johnjbarton yes completely. I don't think dynamic imports should be treated differently to static imports. Code shouldn't randomly collide with other code doing a similar import.

As such, I'd prefer it if the only metadata to the import would be parentName, which could be null, and no address option at all.

[guybedford]

Just to note this is the only issue I don't see assigned that is quite critical.

I've just seen all the other issues have been assigned - I'm really pleased to know that these are being tracked, I wasn't sure if I was going to need to separately file in the ES bug tracker. Many thanks.

[guybedford]

This is now at https://bugs.ecmascript.org/show_bug.cgi?id=2601