Skip to content
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

jspm bundle interoperability with other module loaders #786

Closed
OliverJAsh opened this issue May 22, 2015 · 24 comments
Closed

jspm bundle interoperability with other module loaders #786

OliverJAsh opened this issue May 22, 2015 · 24 comments

Comments

@OliverJAsh
Copy link

At the Guardian, we are migrating our JavaScript projects from RequireJS and curl to SystemJS. We are currently testing SystemJS on a small proportion of users for all JS on theguardian.com. During this transition phase, we require that all JS can be loaded using any one of the these module loaders: RequireJS, curl, and SystemJS.

For the most part, we are OK because all three of these module loaders can interpret AMD modules, which represents the majority of our codebase.

We have found great difficulty when adding a jspm bundle as a dependency. This is because bundles are in the "register" format (System.register), which of course cannot be interpreted by curl or RequireJS. As a work around, we are polyfilling these environments to use SystemJS, so that we can successfully load the jspm bundle.

In my opinion, the output of jspm bundle should be configurable to support different module loaders. If jspm could bundle to AMD, for example, it could be interpreted by all three of the above module loaders.

If I'm developing a library, I might like to author it was a jspm package, but distribute a bundled file of AMD modules for backwards compatibility.

In #75, SFX bundles were decided as the fix to this problem. This does not allow us to export anything, however.

What are your thoughts?

/cc @mattosborn @theefer

@theefer
Copy link

theefer commented May 23, 2015

There is a similar discussion going about the scribe rich-text editing project. In short, we want to migrate projects to ES6 modules and jspm in general to benefit from the goodness of how it manages dependencies, imports any module system, bundles, etc. However, such "jspm setups" seem to make it very hard to consume the resulting project from anything but other "jspm setups". SFX bundles work great but they are not really applicable to libraries (or modules that need to be imported, as @OliverJAsh's case above).

jspm is fantastic to consume any module format, but what's the story for consuming projects managed by jspm?

Is bundling to AMD (or CommonJS or, God forbid, even to globals) a feasible consideration? I know there are issues around circular dependencies, but ignoring that, Traceur supports AMD as a module output format so I'm curious if there is anything systemjs-builder does that prevents such an output?

@dustyjewett
Copy link

100% agree. With React + CJS/Browserify and Ember + ES6/AMD, the framework often chooses the module formats that are supported. Writing a library using jspm that can only output to SystemJS means that only Angular or non-framework projects can use your library.

@guybedford
Copy link
Member

It sounds like the best solution here would be SFX bundles that allow exports and imports, and a custom module format for the SFX bundle itself?

@OliverJAsh
Copy link
Author

How would a loader such as curl or RequireJS be able to interpret the
export? Only AMD would work?

On Tue, 26 May 2015 at 10:54 Guy Bedford [email protected] wrote:

It sounds like the best solution here would be SFX bundles that allow
exports and imports, and a custom module format for the SFX bundle itself?


Reply to this email directly or view it on GitHub
#786 (comment).

@mattosborn
Copy link

Completely agree with what has been said. We've used JSPM for a couple of interactives here but this is a bit of a show stopper for us at the moment because of interoperability between module loaders, and it has resulted in a "polyfill" with nasty performance implications and bad future proofing.

Self-contained bundles that don't pollute the global namespace and allow for a configurable export format would be the ideal solution for us. This would generally make JSPM much more interoperable with other projects too.

@guybedford
Copy link
Member

Sure, so we could allow the SFX bundle to output AMD, CommonJS, ES6 or a global, with exports taking the values from one of the modules in the bundle.

Let me know if that sounds like a sensible solution.

@OliverJAsh
Copy link
Author

That sounds ideal and sensible to me.

On Tue, 26 May 2015 at 11:28 Guy Bedford [email protected] wrote:

Sure, so we could allow the SFX bundle to output AMD, CommonJS, ES6 or a
global, with exports taking the values from one of the modules in the
bundle.

Let me know if that sounds like a sensible solution.


Reply to this email directly or view it on GitHub
#786 (comment).

@guybedford
Copy link
Member

Sure, I've created systemjs/builder#176, let's track progress there.

@guybedford
Copy link
Member

Thanks for suggesting. Will continue to track at the builder repo.

@OliverJAsh
Copy link
Author

Revisting this now and wondering why we can't also have the option to specify an output format for standard bundles (not bundle sfx)?

I ask because we're looking at the option of bundling to AMD and then consuming that using our old module loader (curl) in production, to see if that helps to fix the drop in traffic we've seen when using SystemJS. This would also reduce the payload size of our HTML by 20KB after gzip.

@guybedford
Copy link
Member

@OliverJAsh this could be possible and I completely agree in principle of allowing full flexibility of workflows. The main reason we don't provide this currently is because the focus has been on parity of ES module circular references and bindings, CommonJS execution order, the interaction of circular references between CommonJS circular references and ES module circular references, etc. It could be worth considering this loss of that parity as the cost though, and then ensure code doesn't rely on these edge case behaviours. CommonJS circular references replicate fine in AMD as far as I'm aware so it may not be that critical.

It would be a little work to put together these custom transformers. If you're interested in working on it let me know and I'll gladly provide pointers.

Out of interest - did you try the SystemJS 0.19 upgrade? The bug fixed in systemjs/systemjs#799 may well have improved the IE stats as it was a sporadic failure in IE10 and below. Also if you're able to avoid inlining SystemJS into the page, that is probably preferable so that it is cached after the first load and potentially even add an async attribute to not block the display of HTML. Because of TCP slow start, in theory a critical resource being fetched after the first round trip, while bandwidth is already saturated, shouldn't be a performance cost.

@OliverJAsh
Copy link
Author

It would be a little work to put together these custom transformers. If you're interested in working on it let me know and I'll gladly provide pointers.

I would be interested in working on this. I don't really understand the constraints, so it might help to understand all of this a bit better beforehand:

… ES module circular references and bindings, CommonJS execution order, the interaction of circular references between CommonJS circular references and ES module circular references

Pointers would be very much appreciated 😄. I frequently dip into the code for jspm, SystemJS, and SystemJS builder, so I'm not a total stranger to the codebase, but I wouldn't know where to start other than by looking at how we did this for bundle-sfx in systemjs/builder#185.

Out of interest - did you try the SystemJS 0.19 upgrade? The bug fixed in systemjs/systemjs#799 may well have improved the IE stats as it was a sporadic failure in IE10 and below. Also if you're able to avoid inlining SystemJS into the page, that is probably preferable so that it is cached after the first load and potentially even add an async attribute to not block the display of HTML. Because of TCP slow start, in theory a critical resource being fetched after the first round trip, while bandwidth is already saturated, shouldn't be a performance cost.

I just updated us to 0.19.3 (jspm 0.16.10) earlier today. I'll take another look at the stats once I have about a day’s worth.

Since I last spoke to you about the problem, we have moved our analytics code outside of and above the module loader. Interestingly we're still seeing a drop in traffic. I'm really struggling to think how some JS further down the page, and inside a separate script tag, can have an effect on our analytics. Do you think guardian/frontend#10671 could be causing that somehow?

We have spoken about moving SystemJS from the initial payload and out into a separate file, potentially concatenated with the bundled JS to reduce network requests. I think that's something we should be doing anyway. /cc @sndrs

I really don't like the idea of sacrificing the user experience by adding 20KB (after gzip… 25 million times a day) to the payload (polyfills, SystemJS, config) for improved developer ergonomics. There's nothing to suggest that the size or increased JS execution is the cause of our troubles, but using a micro loader and removing the need for these polyfills is something we would want to do in any case. Bundling to AMD and using a module loader such as curl (which does include lazy loading capabilities) is one way to do that—perhaps we could build a micro loader for System.register (separate from bundle SFX).

@guybedford
Copy link
Member

Well all the circular reference and binding stuff can be blissfully ignored :) The inputs are the raw sources - global, AMD, CommonJS, ES Module or System.register modules (which we'd have to exclude I assume). For each module input, we then need to output a suitable AMD wrapper for it, taking into account the SystemJS configuration for that module (eg globals have exports, CommonJS has globals configuration - again a line can be drawn here on functionality initially as well). The compilers are currently at https://github.com/systemjs/builder/tree/master/compilers, so it would be editing those (or mirroring them) to do the AMD conversion instead of register conversion as a compileOpts.amdBundle option say.

The idea of a micro loader was the goal with the system-register-only build of SystemJS. It may be possible to work on this to make it a fully supported bundling target, and then it is only 6KB of loader as well with the bindings support included. A custom build could be constructed as well to add AMD support to that (and not CommonJS, globals, ES modules etc), to get say an 8KB loader with lazy loading. It would require some consideration of the normalization and config process to ensure things work out as expected.

Let me know if you're interested in the custom optimized SystemJS loader idea, and I can potentially do some prototyping on that as well as it's an area ripe for more polishing that would immediately benefit other users.

That issue is the right slice and would have applied in this case, but wouldn't want to pin too much on it until seeing the numbers - look forward to hearing how they turn out, please keep me posted!

@OliverJAsh
Copy link
Author

The idea of a micro loader was the goal with the system-register-only build of SystemJS. It may be possible to work on this to make it a fully supported bundling target, and then it is only 6KB of loader as well with the bindings support included

What is missing to make it a "fully support bundling target"? Could this be used as is today? I had no idea it existed.

Essentially what we need is a micro version of SystemJS with System.amd{Require,Define} so we can set them as window.{require,define}. Should we open another issue to discuss that?

Alternatively we can just try to use the jspm output (compiled to AMD) with curl which is essentially an AMD micro loader, which is the theme of this GitHub issue thread. I'll go through your pointers above and see how I get on.

@OliverJAsh
Copy link
Author

I have just tried to load jspm AMD SFX bundles using curl. However, any asynchronous requires in the AMD module will run against the SystemJS micro loader contained in the SFX bundle instead of against curl:

<script>
    var curl = {
        paths: {
            foo: 'foo-bundle',
            bar: 'bar-bundle',
        }
    };
</script>
<script src="curl.js"></script>
<script>
    // curl is our AMD loader
    window.require = window.curl;
    console.log(window.require);

    require(['foo'], function (module) {
        console.log(module);
    })
</script>
// foo.js
define([], function () {
    // window.require is overriden by the SystemJS micro loader.
    // Thus, this isn't curl’s require any longer
    console.log(window.require);
    // So this will fail
    require(['bar'], function (module) {
        console.log(module);
    });

    return 'foo module';
});

For this to work, we would have to stop SystemJS from overriding window.require. Ideally jspm would output to pure AMD without the need for the SystemJS micro loader. This would also save us the extra KB of having the micro loader in every AMD bundle we produce (about 10 in total).

@guybedford
Copy link
Member

@OliverJAsh there shouldn't be any conflict caused by removing that. That would be these lines in SystemJS Builder - https://github.com/systemjs/builder/blob/master/templates/amd-helpers.js#L141. We can certainly remove this in the main project as well, as we no longer attempt to bind the dynamic loading environment for SFX bundles.

@guybedford
Copy link
Member

@OliverJAsh
Copy link
Author

Nice. 👍 This will allow us to load AMD SFX bundles with curl, albeit with the overhead of the micro loader in each bundle.

@guybedford
Copy link
Member

Great, glad to hear it's a way forward. Ping me anytime if I can help further.

@OliverJAsh
Copy link
Author

If the test is successful, then we would need to have a way of bundling to AMD without the micro loader. With the micro loader in each bundle, we are incurring a cost of 1.4KB gzipped for each bundle (~10). /cc @sndrs

Will keep you updated.

@guybedford
Copy link
Member

@OliverJAsh the micro loader is there to deal with the interaction of ES modules and other module formats. When a given bundle contains only ES modules or only AMD modules then we can optimize it further.

@OliverJAsh
Copy link
Author

We were wondering if you've got some time today or later this week to chat about the status of our jspm migration? @sndrs has tried updating system builder to allow bundling to AMD, but he's hit a wall with it. Would be good to talk this over with you, if you've got time? 😄

@sndrs
Copy link

sndrs commented Oct 20, 2015

For each module input, we then need to output a suitable AMD wrapper for it, taking into account the SystemJS configuration for that module (eg globals have exports, CommonJS has globals configuration - again a line can be drawn here on functionality initially as well). The compilers are currently at https://github.com/systemjs/builder/tree/master/compilers, so it would be editing those (or mirroring them) to do the AMD conversion instead of register conversion as a compileOpts.amdBundle option say.

I've had a crack at this (which I suspect is maybe too simple, out of ignorance, and doesn't cover globals). It's not behind a flag so it's not PR-ready, but the diff is here: systemjs/builder@master...sndrs:master

The issue i've come up against is that babel helpers from the runtime are being dropped somewhere from the list of runtime dependencies.

I can't workout why or where. es6 runtime stuff is written out (promises etc) but babel's stuff for AMD output (interoprequire etc) is not. That stops the build from working.

I've started tracing the process through compile and trace etc, but got pretty lost :)

Does anything obvious suggest itself? Or is anything obviously wrong with this approach?

@ffflabs
Copy link

ffflabs commented Dec 28, 2015

It took me a bit to realize that jspm-cli is delegating on systemjs-builder internally, so the changes announced in systemjs/builder#176 do apply to jspm as well doing

jspm bundle-sfx src/app.js  dist/app.js --format amd

My two cents!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants