Loader vs ES6 Classes
I've often wondered why we aren't using inheritance to customize Loaders. I'd be interested in the rationale.
In practice we've found that we rarely use the "new Loader(hooks)` option and instead this is more common:
var loader = new Loader(); var loaderFetch = loader.fetch.bind(loader);
loader.fetch = function (loadRecord) { // do something return loaderFetch(loadRecord); };
This way we can think of each modification to the loader in isolation and we can use the same code to modify a custom loader instance and to modify the System instance.
A good point (independently of classes): why change the value of this?
On Aug 4, 2014, at 15:16 , Juan Ignacio Dopazo <jdopazo at yahoo-inc.com> wrote:
In practice we've found that we rarely use the "new Loader(hooks)` option and instead this is more common:
var loader = new Loader(); var loaderFetch = loader.fetch.bind(loader);
loader.fetch = function (loadRecord) { // do something return loaderFetch(loadRecord); };
Why not like this then? You’d need (a compiler for) ES6, though.
class MyLoader extends Loader {
fetch(loadRecord) {
// do something
return super(loadRecord);
}
}
let loader = new MyLoader();
As far as I can tell you are basically arguing that simple Loader hooks don't need object state. Of course that is true.
And sure we can write code that carefully and cleverly avoids using 'this'. Why? ES6 added classes because this is often the clearest way to structure more complex systems.
In my case the LoaderHooks.normalize() function needs to mark names as originating from 'script' rather than 'module' based on the name (eg 'script:' or in my case trailing ',script'. The marking table needs to be on 'this'.
I extend LoaderHooks to InterceptOuputLoaderHooks which calls this.onTranscoded() to copy the transcoded results from load to listeners. But my real point is why should I have to think about 'this' binding in 2015?
We don't need to use an old school API now, we have ES6.
jjb
On Aug 4, 2014, at 16:33 , John Barton <johnjbarton at google.com> wrote:
As far as I can tell you are basically arguing that simple Loader hooks don't need object state. Of course that is true.
No, I’m arguing that Juan’s code is basically “subclassing” a loader, overriding a method and calling that method. ES6 classes seem like a more elegant way of doing this. You’d get as much object state in the subclass as you want.
On Monday, August 4, 2014 11:48 AM, Axel Rauschmayer <axel at rauschma.de> wrote:
On Aug 4, 2014, at 16:33 , John Barton <johnjbarton at google.com> wrote:
As far as I can tell you are basically arguing that simple Loader hooks don't need object state. Of course that is true.
No, I’m arguing that Juan’s code is basically “subclassing” a loader, overriding a method and calling that method. ES6 classes seem like a more elegant way of doing this. You’d get as much object state in the subclass as you want.
We are actually using state a lot for configuration. And there's ongoing discussion about how to do it. So far we've been extending the Loader instance with our own properties (e.g. loader.depCache = {}). The main reason is that we want our extensions to the loader to work both on new instances of the Loader class and on the System loader, which can't be subclassed.
Also, we want each of our extensions to the loader to be self-contained. The reason is that apps that have critical performance needs need to be able to pick the extensions they want instead of bundling everything in case they need them. Subclassing doesn't quite get along with this approach. We also expect performance critical applications to insert their loader extensions in the HTML page or to PUSH them using SPDY. This way, in browsers that have the Loader API, we avoid this situation:
Fetch HTML page -> Fetch Loader extensions -> Fetch the app's code
Some links to current discussion:
- Conditional loading configuration: systemjs/systemjs#126
- Reconsidering extension process again: systemjs/systemjs#158
- Package configuration: systemjs/systemjs#141
Neither example binds 'this' assuming the current Loader implementation, so no object state. Although maybe you are saying your example ought to work, then we agree ;-)
We seem to have a different understanding of “object state”. What do you mean by that term?
We are actually using state a lot for configuration. And there's ongoing discussion about how to do it. So far we've been extending the Loader instance with our own properties (e.g. loader.depCache = {}). The main reason is that we want our extensions to the loader to work both on new instances of the Loader class and on the System loader, which can't be subclassed.
Curious: why can't the system loader be subclassed? I get that System
is an instance, but why can't you subclass whatever the System instance
is based on?
Does prototypal inheritance not work on the System instance?
On Sun, Aug 3, 2014 at 2:31 PM, Kevin Smith <zenparsing at gmail.com> wrote:
I've often wondered why we aren't using inheritance to customize Loaders. I'd be interested in the rationale.
When I was working on Loaders, I modified the design so that inheritance works.
That is, you can just subclass Loader and declare the methods you want to override, and the Loader will call those methods. It all works, as far as I can tell.
I don't remember why we kept the options argument after that. I'm in favor of dropping it.
I suppose the point is whether System can be subclassed itself, since that is usually the starting point for a new loader as it is common to share the base environment normalization, locate and fetch functions.
Currently, if System is set via the options hooks this isn't possible.
Note also that things like baseURL and the mapping table being created through the constructor would also make sharing these easier.
If System were defined to be based on a class found at System.constructor like this, things get a lot easier for that starting point.
IMO a better design would have each platform subclass Loader, eg SystemLoader extends Loader and System (or 'system' if we decide to suddenly come to our senses) would be an instance of SystemLoader. SystemLoader would define the hook functions and custom Loader classes would extend SystemLoader to add features. Custom loaders probably don't want to re-implement the hooks so much as augment them. The SystemLoader would support this naturally.
IMO a better design would have each platform subclass Loader, eg SystemLoader extends Loader and System (or 'system' if we decide to suddenly come to our senses) would be an instance of SystemLoader. SystemLoader would define the hook functions and custom Loader classes would extend SystemLoader to add features. Custom loaders probably don't want to re-implement the hooks so much as augment them. The SystemLoader would support this naturally.
+1
Also +1 to renaming "System" something more sensible.
On Mon, Aug 4, 2014 at 3:23 PM, John Barton <johnjbarton at google.com> wrote:
IMO a better design would have each platform subclass Loader, eg SystemLoader extends Loader and System (or 'system' if we decide to suddenly come to our senses) would be an instance of SystemLoader. SystemLoader would define the hook functions and custom Loader classes would extend SystemLoader to add features. Custom loaders probably don't want to re-implement the hooks so much as augment them. The SystemLoader would support this naturally.
I agree with the broad strokes here.
The design as it stands doesn't specify the system loader or its class. It's a platform-specific detail.
On Mon, Aug 4, 2014 at 1:50 PM, Jason Orendorff <jason.orendorff at gmail.com>
wrote:
The design as it stands doesn't specify the system loader or its class. It's a platform-specific detail.
Yes, but ES should say a couple of things to get everyone on the same page, eg that the system loader instance will have the interface and semantics of Loader and be available on Reflect.global (or whatever).
jjb
Would the System object for a module loaded with a sub classed System be the sub classed one or the original one? IMO a better design would have each platform subclass Loader, eg SystemLoader extends Loader and System (or 'system' if we decide to suddenly come to our senses) would be an instance of SystemLoader. SystemLoader would define the hook functions and custom Loader classes would extend SystemLoader to add features. Custom loaders probably don't want to re-implement the hooks so much as augment them. The SystemLoader would support this naturally.
The System object is global, so "depends" is the answer to all such questions.
Sorry for the top post. Can someone provide a concrete example of how ES6 classes fall down here?
If I understand correctly, John was wanting to use a state-carrying options as the options argument when specifying hook overrides. I think the best solution would be to simply remove the options argument altogether (unless there's still a good reason to keep it).
On Mon, Aug 4, 2014 at 6:33 PM, Kevin Smith <zenparsing at gmail.com> wrote:
If I understand correctly, John was wanting to use a state-carrying options as the options argument when specifying hook overrides.
What part of this has anything to do with ES6 classes?
As far as I can tell, nothing. Looks like good work was done to make Loader class-friendly.
f you read the Loader source carefully you can see that Jason fixed the Loader to support inheritance. But his fix does not allow classes to inherit the platform's loading behavior. That behavior is given in options properties passed to instances. By defining the system loader as an instance of a SystemLoader extending Loader, the platform behavior would be extendable and the Loader would be ES6 not merely class-friendly.
Related issue that had come up in discussions I've had off this list: is it likely that the spec will constrain the constructor arguments of custom Loader classes to match the arguments of Reflect.Loader?
For my part I was surprised that this might be a possibility.
jjb
Since I guess not too many developers work with ES6 and the Loader object, here is some feedback: the Loader callback design does not play well with ES6 classes.
The Loader takes 'options', an object with function properties like normalize, locate, and fetch. If you pass a literal object with function properties, it works fine.
If you pass an instance of a class, then you discover that the Loader calls these functions with 'this' bound to the Loader, not the 'options' object.
There isn't a simple way around this as far as I know. The options functions don't have access to the options object, only module-state and global. So you're stuck with the awkward: var loader = new Loader({ normalize: options.normalize.bind(options), locate: options.locate.bind(locate), etc, }; I guess you can give up on ES6 inheritance and create the hook instances by old-school JS, but that is even more annoying. Or maybe there is something else I've not thought of?
jjb
Since I guess not too many developers work with ES6 and the Loader object, here is some feedback: the Loader callback design does not play well with ES6 classes. The Loader takes 'options', an object with function properties like normalize, locate, and fetch. If you pass a literal object with function properties, it works fine. If you pass an instance of a class, then you discover that the Loader calls these functions with 'this' bound to the Loader, not the 'options' object. There isn't a simple way around this as far as I know. The options functions don't have access to the options object, only module-state and global. So you're stuck with the awkward: var loader = new Loader({ normalize: options.normalize.bind(options), locate: options.locate.bind(locate), etc, }; I guess you can give up on ES6 inheritance and create the hook instances by old-school JS, but that is even more annoying. Or maybe there is something else I've not thought of? jjb -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20140803/5b0c68fe/attachment-0001.html>