1.0 Spec Rewrite - Work in Progress - #317
Conversation
|
Note it's way easier to review this in the branch - https://github.com/ModuleLoader/es6-module-loader/tree/1.0/src |
|
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? |
|
I'd suggest adding a comment to system.js that this corresponds to section 8 of the spec |
|
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. |
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. |
|
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.
|
888f2bc to
a65478f
Compare
There was a problem hiding this comment.
is this intentional having 'm .' rather than 'm.' throughout this function?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think this is only being used as a lookup, right? If so should really be an object instead.
There was a problem hiding this comment.
Thanks. Yes that would be a great point for perf critical, but this function is only called a handful of times.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yes this is what is currently defined. Do share your feedback on the spec repo if appropriate.
6a81827 to
78e0f8e
Compare
dcda0b5 to
2695ef8
Compare
|
Updated to whatwg/loader@2946478. NB still need to port from 69b2589 on master. |
a34508c to
fcb809a
Compare
|
The branches have now diverged, so closing this PR. The 1.0 branch will be continually actively developed now. |
|
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? |
|
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. |
|
ok, I'll try some 'experimentation' :-) |
|
@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? |
|
@matthewp good question - |
|
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. |
|
AFAICS |
|
The spec says |
No description provided.