New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in import function of webpack plugin #15

Closed
patroza opened this Issue Jul 28, 2016 · 9 comments

Comments

Projects
None yet
2 participants
@patroza

patroza commented Jul 28, 2016

There's a bug in the webpack loader _import implementation,
when you require the same resource multiple times from within the same template, then the import function will return an object on all subsequent calls, and only the first call will resolve properly.

Viewmodel error will be: No view model found in module "..."
Template error will be: Template markup must be wrapped in a <template> element e.g. <template> <!-- markup here --> </template>

my workaround:

return new Promise(function (resolve, reject) {
...

->

WebpackLoader.map = {};
...
return WebpackLoader.map[moduleId] || ( WebpackLoader.map[moduleId] = new Promise(function (resolve, reject) {
... );
@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Jul 28, 2016

Member

Can you post that workaround again, seems something got lost, I can't make out what's the code supposed to do? Thanks

Member

niieani commented Jul 28, 2016

Can you post that workaround again, seems something got lost, I can't make out what's the code supposed to do? Thanks

@patroza

This comment has been minimized.

Show comment
Hide comment
@patroza

patroza Jul 28, 2016

@niieani I am caching the promise based on moduleId, so that on re-entry of the import method, the cached instance is being re-used.

(So far what I believe is going wrong is that the second time the request is made, it is made without a loader specified)

I hope I can find some time to make a repro project for you, it's just really busy atm..

patroza commented Jul 28, 2016

@niieani I am caching the promise based on moduleId, so that on re-entry of the import method, the cached instance is being re-used.

(So far what I believe is going wrong is that the second time the request is made, it is made without a loader specified)

I hope I can find some time to make a repro project for you, it's just really busy atm..

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Jul 28, 2016

Member

I think that's a race condition, where certain variables are already set when the second loading is being done. Can you post the full code of your workaround, please?

Member

niieani commented Jul 28, 2016

I think that's a race condition, where certain variables are already set when the second loading is being done. Can you post the full code of your workaround, please?

@patroza

This comment has been minimized.

Show comment
Hide comment
@patroza

patroza Jul 28, 2016

@niieani

inside the dist\commonjs\aurelia-loader-webpack.js

  WebpackLoader.modulePromiseMap = {};

  WebpackLoader.prototype._import = function _import(moduleId) {
    var _this2 = this;

    var moduleIdParts = moduleId.split('!');
    var path = moduleIdParts.splice(moduleIdParts.length - 1, 1)[0];
    var loaderPlugin = moduleIdParts.length === 1 ? moduleIdParts[0] : null;

    return WebpackLoader.modulePromiseMap[moduleId] ||
               (WebpackLoader.modulePromiseMap[moduleId] =
                 new Promise(function (resolve, reject) {
      try {
        if (loaderPlugin) {
          resolve(_this2.loaderPlugins[loaderPlugin].fetch(path));
        } else {
          try {
            var result = __webpack_require__(path);
            resolve(result);
            return;
          } catch (_) {}
          require.ensure([], function (require) {
            var result = require('aurelia-loader-context/' + path);
            if (typeof result === 'function') {
              result(function (res) {
                return resolve(res);
              });
            } else {
              resolve(result);
            }
          }, 'app');
        }
      } catch (e) {
        reject(e);
      }
    }));
  };

patroza commented Jul 28, 2016

@niieani

inside the dist\commonjs\aurelia-loader-webpack.js

  WebpackLoader.modulePromiseMap = {};

  WebpackLoader.prototype._import = function _import(moduleId) {
    var _this2 = this;

    var moduleIdParts = moduleId.split('!');
    var path = moduleIdParts.splice(moduleIdParts.length - 1, 1)[0];
    var loaderPlugin = moduleIdParts.length === 1 ? moduleIdParts[0] : null;

    return WebpackLoader.modulePromiseMap[moduleId] ||
               (WebpackLoader.modulePromiseMap[moduleId] =
                 new Promise(function (resolve, reject) {
      try {
        if (loaderPlugin) {
          resolve(_this2.loaderPlugins[loaderPlugin].fetch(path));
        } else {
          try {
            var result = __webpack_require__(path);
            resolve(result);
            return;
          } catch (_) {}
          require.ensure([], function (require) {
            var result = require('aurelia-loader-context/' + path);
            if (typeof result === 'function') {
              result(function (res) {
                return resolve(res);
              });
            } else {
              resolve(result);
            }
          }, 'app');
        }
      } catch (e) {
        reject(e);
      }
    }));
  };
@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Jul 28, 2016

Member

Thank you. I'll try to get this fixed soon.

Member

niieani commented Jul 28, 2016

Thank you. I'll try to get this fixed soon.

@patroza

This comment has been minimized.

Show comment
Hide comment
@patroza

patroza Jul 28, 2016

@niieani thanks a lot. I had the issue before with CSS files since the RC, and since the Stable it happens on all files, somehow that's a relief :)
I also wonder if this might've been the root cause of my other issue before (can't call call of undefined when running with the multi chunks)

patroza commented Jul 28, 2016

@niieani thanks a lot. I had the issue before with CSS files since the RC, and since the Stable it happens on all files, somehow that's a relief :)
I also wonder if this might've been the root cause of my other issue before (can't call call of undefined when running with the multi chunks)

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Jul 28, 2016

Member

Good that we're tracking this then. Could you give me a a short snippet of how I could reproduce this in testing?

Member

niieani commented Jul 28, 2016

Good that we're tracking this then. Could you give me a a short snippet of how I could reproduce this in testing?

@niieani niieani closed this in 26e1a74 Jul 28, 2016

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Jul 28, 2016

Member

A release with this in will be out shortly.

Member

niieani commented Jul 28, 2016

A release with this in will be out shortly.

@patroza

This comment has been minimized.

Show comment
Hide comment
@patroza

patroza Jul 30, 2016

@niieani thanks, however now this one is back :(

aurelia/skeleton-navigation#613 (comment)

(I dont know if it ever was really away, as it just appears and disappears from compile to compile :))

patroza commented Jul 30, 2016

@niieani thanks, however now this one is back :(

aurelia/skeleton-navigation#613 (comment)

(I dont know if it ever was really away, as it just appears and disappears from compile to compile :))

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