Server-side/CommonJS loading time increase

[mmahalwy]

This works great when on the client but I noticed on the server it adds more load time when requiring large components that have many children (nested components / libs). I should mention that server I mean using CommonJS

Thoughts?

[thgreasi]

Yes, that's expected, since as demonstrated in README, it attempts to change the synchronous nature of the the CommonJS require() call, by wrapping it inside a promise resolve(require('./utils/serializer')); . If you need to have symmetric JS, in order to run your modules on both the client and the server, then you:

• either use es6 imports and forgo dynamic loading
• or use this plugin and have the aforementioned Promise penalty when targeting CommonJS

To be honest, the primary use case that I had in mind while writing this, was to be used by library creators along with the UMD pattern, as a modern way to forget require() and support RequireJS asynchronous loading (and as an extra, making it easier to swap between the different bundling platforms). Lastly, If you are primarily targeting only server side code, then I would suggest you to use es6 imports, since you don't really need dynamic loading in this case.

[thgreasi]

@mmahalwy I'm closing this since as far as I can thing at the moment, there isn't anything that we could do to improve this. On the other hand I'm open to suggestions. eg: if the target is just CommonJS, then we could hackishly replace the code with a method call with an object having a dummy a then() method (but with a second thought, such a thing doesn't feel too robust).

[thgreasi]

If Promise.resolve() is faster than a Promise Constructor, then an other possible approach could look like this:

(function() {
    var global = window;
    if (typeof global.define === 'function' && global.define.amd) {
        return new Promise(function (resolve, reject) {
            return global.require(['utilsSerializer'], resolve, reject);
        });
    } else if (typeof module !== 'undefined' && (module.exports && typeof require !== 'undefined') ||
               typeof module !== 'undefined' && (module.component && (global.require && global.require.loader === 'component'))) {
        return Promise.resolve(require('./utils/serializer'));
    } else {
        return Promise.resolve(global['utilsSerializer']);
    }
})();

[mmahalwy]

@thgreasi Thanks for the extensive thought into this! To address your comments.

1. I think isomorphic (universal) javascript apps are becoming popular and so the challenges of server-side code and client side code is a problem
2. Webpack 2 is not fully out yet but it's awesome and will be a game changer. Especially with code splitting with System.import
3. This library is awesome. I cannot live without it.
4. I think your suggestion of using Promise.resolve is a great solution. I wonder also if there is a way to just fall back on require() and figuring out a way to remove the Promise code. For example, see https://github.com/knpwrs/babel-plugin-remove-webpack . Both good libraries but I think they stopped supporting them. At least, I think on the server/node env level, it might be a good suggestion to replace System.import('./module').then(modue => module) with require('./module'). Might be tricky - I have limited knowledge in creating babel plugins

[thgreasi]

Changing asynchronous Promise code to a synchronous require() can't be done 1-1 and without drastic changes to the source, which might also change the meaning of the original code.

The case that such change might be safer is when es7 await is used, since the code already has synchronous-like form. Maybe we can add an optimization level flag and apply such a change when Global or CommonJS is used.

Thodoris Greasidis Computer, Networking & Communications Engineer

[thgreasi]

Just did some performance tests about using Promise.resolve() wherever possible (CommonJS & Global). It seems that we might be able to get a speed-up of approximately 18% on nodejs and 11-28% on browsers. (To be clear, this just reduces the penalty of the Promise nature that System.import defines.)

I still think that eliminating the Promises penalty might be very difficult, but I'm open to suggestions.

[mmahalwy]

@thgreasi seems like that's a great first step! Moving to Promise.resolve() would be huge for Commonjs since it'll reduce the server rendering by 18% right off the bat ;)

[thgreasi]

Just pushed a new branch and opened #6 as the first step of this issue. Can you please give it a try? If it works for you then I'm planning to add some tests (currently I test it against localforage) and release it as v2.1.0 . The next step would to try replace some await System.import() expressions for non-amd targets.

[mmahalwy]

Sounds good. Will try it tonight

[mmahalwy]

@thgreasi 2.1.0 coming out soon?

[thgreasi]

I'm just waiting for your gogo signal to release it.

If it works for you then I'm planning to add some tests (currently I test it against localforage) and release it as v2.1.0 .

[mmahalwy]

@thgreasi Okay so I downloaded the package directly from github via

"babel-plugin-system-import-transformer": "git@github.com:thgreasi/babel-plugin-system-import-transformer.git",

And starting my server I got error:

[piping] error given was: ReferenceError: Unknown plugin "system-import-transformer" specified in "base" at

Any suggestions?

[mmahalwy]

Noticed it's not downloading anything

> ls node_modules/babel-plugin-system-import-transformer/
LICENSE      README.md    package.json

[thgreasi]

How about just cloning inside your node modules folder?

Thodoris Greasidis

[thgreasi]

Oh, and you should also do a:

npm run-script build

Thodoris Greasidis

[mmahalwy]

Works and looks good to me! I don't have a benchmark of how much faster but it definitely feels faster.

[thgreasi]

Great, I will try to release it tonight!

Thodoris Greasidis

[thgreasi]

Just released v2.1.0. I'm already planning an other optimization approach under an optimization flag. This time I will try to work around the async nature of the then() method, as the spec defines it. I'm thinking about something like this:

(function() {
    var lib = require('lib');
    var promise =   Promise.resolve(lib);
    var oldThen = promise.then;
    promise.then = function(success) {
        return Promise.resolve(typeof success === 'function' ?
            success(lib) :
            oldThen.apply(this, arguments));
    };
    return promise;
})();

This way the callback provided to the then() method is immediately invoked. Any thoughts?

[mmahalwy]

@thgreasi this looks awesome. I don't know much about

    var lib = require('lib');
    var promise =   Promise.resolve(lib);

I have to look into that library to understand what it does.