Implement SystemLoader constructor

[matthewp]

This adds a SystemLoader constructor function used internally to inherit from Loader. System then becomes an instance of SystemLoader and SystemLoader is set as System.constructor. This way downstream code can do something like:

class MyLoader extends System.constructor {

  locate(load) {
    return super(load).then(function(address) { ... });
  }

}

And doing so will have access to super and the like.

I also added in esnext to build this and use es6 classes. We talked about doing it this way and it is much more elegant in my opinion (just look at the minimal amount of code changes).

Let me know if this adds too much code or you don't like it for some reason and I'll implement this as regular old constructor functions, but I thought this turned out really well.

One thing to consider is that the tests were pointing directly to lib/loader.js and lib/system.js so I made a built version of each of those go to dist/ as well.

Review this carefully because the requirements were spread out over various issues and personal discussions.

Fixes #199

[guybedford]

I'm not sure we need to call super against this?

It may be worth still keeping constructor(options) and then running super(options).

[matthewp]

Yep, you're right, I'll update this.

[guybedford]

This is great! Works out really elegantly yes and very happy with it.

Perhaps lets rename lib to src and dist to lib? That way we retain the same file structure we have already. Then it would be nice to keep the current dist as well so that users can easily get the complete bundled loader. It's also great for backwards compat.

[matthewp]

I think we can keep index.js, and the built versions of loader.js and system.js in lib. Put the source (for es6 files) in src and keep dist the way it is. Make sense?

[guybedford]

Sounds great.

[matthewp]

Hey @guybedford , I think i've accounted for all of your questions, but please do review thoroughly again.

Here are the tests passing in IE9 (don't have an IE8 available):

[matthewp]

@guybedford I still don't love the way I'm patching the Object.* functions. I think it would be better to actually polyfill the Object.* functions themselves. I don't know how you feel about adding more polyfills though.

The advantage is that we could stick these in the top of loader.js and any other project that uses esnext (SystemJS, other forks, extensions, etc.) wouldn't have to worry about patching it themselves.

Let me know if that sounds good to you and I'll make the change. Then we could eliminate the string-replace step entirely.

[guybedford]

We should do this in the outer function - otherwise the try - catch is going to deoptimize the defineProperty function everytime it is called. Note we should still wrap it in a function closure so the try - catch doesn't deoptimize the whole script.

[guybedford]

I'd prefer not to polyfill Object, since it may mess with other scripts etc. This way we know we're not meddling with other stuff, and not handling this layer at all apart from internally.

Will do the IE8 testing myself no problem... happy to go with it with the last perf adjustment.

[guybedford]

There is actually a defineProperty already defined here - https://github.com/ModuleLoader/es6-module-loader/blob/master/lib/loader.js#L110. So we should make sure we share the implementation (perhaps just assign $__Object$defineProperty = defineProperty if that works).

[matthewp]

That won't work, that defineProperty is inside the loader polyfill's closure. So I'll have to move it outside and wrap both of them in a single closure.

[guybedford]

Ah right... it may be worth doing... or is that too many closures?

[matthewp]

There's no such thing as too many closures ;) I'll get it updated tonight.

[guybedford]

Alright! Since we're not compiling or loading the scripts separately anymore it may be worth sharing the outer closure anyway. For example, __eval has to run in the outermost closure to avoid accidental variable binding.

[matthewp]

All, I think this gets everything. To summarize:

1. loader.js and system.js get ran through esnext and compiled into a single file.
2. string-replace is ran to remove the $__Object$getPrototypeOf, etc. from them (we are going to put our own in).
3. We concat polyfill-wrapper-start.js (which is the outermost closure which contains the above polyfill functions), the result of the loader/system compilation, and polyfill-wrapper-end.js into dist/es6-module-loader.js
4. loader.js and system.js both lose their own closures and are instead sharing a single one. This means __eval is in the outermost closure like you said it needs to be.

Does this cover everything? Oh, all tests still pass.

[guybedford]

I just adjusted the wrappers slightly to ensure the eval doesn't get any of the local variables leaked.

Thanks so much for this - SystemJS tests are passing against it too which is great. Will be nice to write proper extensions now.