1.0 Spec Rewrite - Work in Progress

[guybedford]

No description provided.

[guybedford]

Note it's way easier to review this in the branch - https://github.com/ModuleLoader/es6-module-loader/tree/1.0/src

[probins]

a major change! You're calling this 1.0 - does this mean you regard the spec as stable? I see quite a lot of Todos in it, which implies that there's still quite a lot to do before it's finished.

Does the change from module names to urls/addresses mean that System.paths is no longer needed?

[probins]

I'd suggest adding a comment to system.js that this corresponds to section 8 of the spec

[guybedford]

The release plan is to start with an alpha, then to release a series of betas, before the 1.0.0 stable. These prerelease phases will happen over the coming months. Then there's no rush to the 1.0.0. The spec API is actually pretty small so that there isn't a high risk in a 1.0.0 at all. We'll then allow for new major versions as corrections to the spec, aiming to reach stability before 10.

[probins]

Does the change from module names to urls/addresses mean that System.paths is no longer needed?

looks like System.paths is replaced by site.mappings. The docs will have to be changed to reflect this. Not sure how you do that with the wiki, as wiki is not version-specific. AIUI, module-tag.js and parser.js are optional, and can be left out if you're not loading ES6 modules. If that's correct, this should also be documented in the Readme.

[guybedford]

Yeah that's an issue with using the Wiki for docs. Once the change is through though shouldn't be as much of an issue - we can keep all the existing info in a legacy documentation somewhere.

module-tag.js and parser.js can be removed but it's only a 1KB saving, and for simplicity of use I think it's worth just leaving them in. It retains the full polyfill meaning with expected results that way.

[probins]

is this intentional having 'm .' rather than 'm.' throughout this function?

[guybedford]

Thanks for taking a look. Yes it is because I want the loader itself loadable through SystemJS, which detects the register format in code as the existence of the "System.register" string in source, as the highest priority format.

[matthewp]

I think this is only being used as a lookup, right? If so should really be an object instead.

[guybedford]

Thanks. Yes that would be a great point for perf critical, but this function is only called a handful of times.

[matthewp]

Is this really all that hook does? This is disappointing. I don't see the point, actually. I thought/hoped hook somehow managed having multiple functions for the same hook.

[guybedford]

Yes this is what is currently defined. Do share your feedback on the spec repo if appropriate.

[guybedford]

Updated to whatwg/loader@2946478.

NB still need to port from 69b2589 on master.

[guybedford]

The branches have now diverged, so closing this PR.

The 1.0 branch will be continually actively developed now.

[probins]

how usable do you regard 1.0 branch? Should I switch over to using it now, or would it be better to hold off for a while?

[guybedford]

It would be great to get feedback, but it's just for experimentation still. I'll aim to get an alpha out as soon as it makes sense. Even then no rush to upgrade.

[probins]

ok, I'll try some 'experimentation' :-)

[matthewp]

@guybedford I still don't understand why there is a System. I know it sounds pedantic but there is no System any more, in the new spec. SystemJS can still alias window.System to Reflect.loader so there's no breakage. I guess I just don't understand why this project needs to have non-spec properties. Or is this just a temporary measure because you were more focused on implementing the important parts of the new spec?

[guybedford]

@matthewp good question - Reflect.loader doesn't seem that set yet. These important points do still seem unclear so these are the sorts of things we're waiting on before an alpha.

[matthewp]

Ok, I understand that perspective. So far you've been focusing on the big parts that are unlikely to change (unless I have my way ;) ), so stuff like naming can be iterated on. Fair enough.

[probins]

AFAICS Reflect.loader (as opposed to Reflect.Loader) isn't in the current spec either

[matthewp]

The spec says Reflect.Loader but in the issues its been agreed that Reflect.Loader will be the constructor (there isn't one in the spec now) and something else, probably Reflect.loader will be the default instance. But what isn't in the spec at all, is the word System. :-)