Server-side/CommonJS loading time increase about babel-plugin-system-import-transformer HOT 19 OPEN

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']);
    }
})();

from babel-plugin-system-import-transformer.

@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

from babel-plugin-system-import-transformer.

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

from babel-plugin-system-import-transformer.

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.

from babel-plugin-system-import-transformer.

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.

from babel-plugin-system-import-transformer.

@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).

from babel-plugin-system-import-transformer.

@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 ;)

from babel-plugin-system-import-transformer.

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.

from babel-plugin-system-import-transformer.

Sounds good. Will try it tonight

from babel-plugin-system-import-transformer.

@thgreasi 2.1.0 coming out soon?

from babel-plugin-system-import-transformer.

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 .

from babel-plugin-system-import-transformer.

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

"babel-plugin-system-import-transformer": "[email protected]: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?

from babel-plugin-system-import-transformer.

Noticed it's not downloading anything

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

from babel-plugin-system-import-transformer.

How about just cloning inside your node modules folder?

Thodoris Greasidis

from babel-plugin-system-import-transformer.

Oh, and you should also do a:

npm run-script build

Thodoris Greasidis

from babel-plugin-system-import-transformer.

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

from babel-plugin-system-import-transformer.

Great, I will try to release it tonight!

Thodoris Greasidis

from babel-plugin-system-import-transformer.

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?

from babel-plugin-system-import-transformer.

@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.

from babel-plugin-system-import-transformer.