names for Loader constructor and default loader

[dherman]

Summary

Decide on names for two things: the Loader constructor and the default loader instance.

Background

Originally we had global System and Loader but that was when we expected System.get() to be a common use case. Once we worked out dynamic relative import, the default loader became a much more low-level API: it's meant for frameworks and loader customization logic, not for everyday use cases. So IMO giving it an attractive global name like System is inappropriate. Also, it was kind of competing with window, navigator, and process as Yet Another representation of the top level application state.

IMO Reflect.Loader is good for the constructor, but this still leaves open the question of where the default loader should live. Suggestions:

• another name in Reflect:
  • Reflect.System
  • Reflect.LOADER
• a static property or method of Reflect.Loader:
  • Reflect.Loader.DEFAULT
  • Reflect.Loader.current
  • Reflect.Loader.current()
• Reflect.Loader itself is both the constructor and the instance

My preference is a Reflect.Loader.current getter. ALLCAPS feels more appropriate for immutable constants like Math.PI rather than a mutable object, and a current() method feels like overkill for just accessing a singleton object. But I like the idea that it's conceptually associated with the Loader type but it's a static singleton instance.

Drawbacks

Systemjs already has named itself after System. But it's still a pretty cool name for the project, and this just gives it a more fun historical etymology. :)

[matthewp]

What's wrong with loader? It's descriptive enough to me.

I don't like current because I would expect Reflect.Loader.current to be the current loader in multiple loader environments. I would expect it to work the same as document.currentScript. We do need this capability, btw, but that's for another discussion.

I would strongly favor it being lower-case whichever name is chosen, it's too hard to forget it's not a constructor otherwise.

[domenic]

Reflect.Loader.default seems nice and wasn't mentioned.

[dherman]

I like Reflect.Loader.default a lot!

I think having Reflect.Loader and Reflect.loader both is too confusing, and Reflect.Loader.loader is just a little WAT.

[caridy]

I don't like current because it implies something that is changing overtime, which is probably not the case.

I think default its gonna be confusing for some people, I can imagine questions like: "is Loader.default to load things exported thru export default?"

I don't have a better suggestion. /cc @ericf

[matthewp]

I think having Reflect.Loader and Reflect.loader both is too confusing

Not to be argumentative, but why? It's pretty common to name constructor/instances in this way.

I don't hate default either. My only qualm would be that you tend to use the loader more than you realize so Reflect.Loader.default is a lot of typing.

[dherman]

Not to be argumentative, but why?

Argumentativeness welcome ;-)

It's pretty common to name constructor/instances in this way.

Fair point. I'm not sure I can justify my intuition. And maybe Realm will become the home of several realm-global variables: Reflect.realm, Reflect.global, Reflect.loader. I'm open to it.

[caridy]

I really like Reflect.loader (alongside Reflect.realm and Reflect.global). Mostly because it will be very friendly for people who don't care about loader customization, they will simply do Reflect.loader.site({...}); Reflect.loader.import(...);.

[arv]

Reflect.Loader.default or Reflect.defaultLoader.

I am also a bit concerned about Reflect.Loader vs Reflect.loader. Too easy to misspell.

[caridy]

Let me clarify my position, I want to see a very simple way to access the default loader without having to know that you can create more than one loader (as an advanced feature). Reflect.loader in lower case seems to be the simplest, and it aligns well with Reflect.global and Reflect.realm. From there, the question is, how to access the constructor? Reflect.loader.constructor maybe?

[matthewp]

@caridy The constructor is Reflect.Loader no? Reflect.loader instanceof Reflect.Loader should be true. We've made a mistake somewhere if not.

[dherman]

@matthewp I think he means the most derived constructor, in the case where the host environment decided to make Reflect.loader be an instance of a host-specific subclass of Reflect.Loader.

[matthewp]

@dherman Ah, yes, then as is normally the case, Reflect.loader.constructor will point to the most derived constructor. If you are working in an environment which overwrites Reflect.loader this is where the extension point would be.

[guybedford]

@dherman thanks for the SystemJS consideration, but don't let it be a drawback! Quite happy to have an etymological name.

[dherman]

@caridy I think your position is the strongest. There's plenty of precedent for lowercase singletons sitting next to their uppercased constructor (window instanceof Window, document instanceof Document), and in each case you can work with the singleton without needing to know how to use the constructor.

I think we should go with Reflect.Loader and Reflect.loader. I'll update the spec soon.

[nevir]

+1 to Reflect.loader (or, a sacrilegious suggestion of just loader, __loader, etc).

Side question: Is there some background on why Reflect is the right place to hang this off of? The loader seems to have no relationship to reflection or any of that; seems weird.

[dherman]

I think it's pretty appropriate. Getting to muck with how code is loaded s getting to "peek under the hood" of the system. One of the goals of the Reflect object is to do a better job than JS historically has done about clearly segregating low-level operations that expose or intercede the inner workings of the system from ordinary everyday operations.

[caridy]

update: we need to discuss this with @erights, according to @dherman, there are some valid concerns.

[matthewp]

What are the concerns? @erights?

[caridy]

@matthewp Reflect.loader is not reflective :)

[erights]

Reflect does not provide any ability to cause I/O. If transitively frozen, it provides no access to mutable state, and so can be shared among subgraphs in the same realm that should not be able to communicate.

The loader is the third example of an (existing or proposed) primordial object that does grant authority:

• The global object, for which we rejected Reflect.global for the same reason.
• The proposed WeakReference factory (e.g., WeakRef or makeWeakRef) which makes GC observable, opening a covert channel. This is why we carefully separated WeakMap from any notion of WeakRef.
• The default loader, which causes external I/O and internally has a mutable mapping from module names to mutable module instances.

We do need a good way to provide standard access to authority-providing primordials such as these. In particular, I think all three of these are well motivated and should somehow become standard. But, because they provide authority they should be clearly separated from the means of providing abstractions that are safely sharable. I did not love having Loader on System, but I did not object because the separation was clear.

[caridy]

Once we decide a way to provide access to the "default loader", we can clean up the current document to explicitly reference to Reflect.Loader class, and define a clear separation with <TBD-default-loader>, which is ambiguous today.

[erights]

explicitly reference to Reflect.Loader class

Perhaps I misunderstood something. From the API doc, it looked to me like Reflect.Loader was the default loader, not a class for making loaders. It is worth pointing out that a class (or any kind of factory) that, given functions for all four traps, makes a fresh loader, would not itself provide any authority. But in that case, of course it would not directly support APIs like Reflect.Loader.import(name[, referrer]). The import method (and the others) would be loader instance methods, not static loader class methods.

[matthewp]

We need both a Loader constructor and a default loader. The docs haven't been updated to reflect (pun intended) this yet. Waiting on a decision on the names, I guess.

[erights]

If the Loader constructor is authority-free, then I have no objection to having it on Reflect. I might even prefer it. AFAICT, for it to be authority free, all four traps would need to be provided in order for it to make a Loader instance. Does that seem right?

[caridy]

correct. that is why we need the separation. I will go ahead and update the doc to reflect that. For now I will use loader for the default instance of Reflect.Loader. We can change it later if needed.

[dherman]

@erights This makes some sense to me, not only from the POLA perspective but also as a reasonable grouping: the Reflect namespace can be thought of as pure operations that can reflect on stuff, and we could put the stuff that connects to the host environment under a join namespace. I'm even happy with bringing back System for this purpose but purely as a namespace under which these objects live. To wit, something like:

System.global // global object
System.loader // this realm's loader
System.registry // this realm's module registry

The constraints here are:

• putting the loader and registry underneath a namespace messages that they're not as common-use as global variables like Array and document
• grouping all the objects that give access to the host environment / high privilege operations sections them off so that only that one namespace gets censored in a frozen / low privilege realm
• naming the namespace such that it is suggestive of providing access to facilities of the system

Thoughts?

[dherman]

As for Reflect.Loader, I believe it should be effect-/authority-free, but it's also not immediately clear there's much functionality left to justify its existence. Maybe @guybedford has some implementation experience about how much logic it actually abstracts—Guy, do you have an implementation of the Loader constructor you could link to?

[domenic]

Definite +1 for going back to System and leaving Reflect for meta-object protocol stuff :)

[erights]

I like this a lot, thanks!

[matthewp]

@dherman maybe I'm misunderstanding you but are you suggesting getting rid of the Loader constructor? I believe we debated this a while back and it was agreed to bring it back and to put the hooks on the prototype for easier extensibility.

[matthewp]

Here's a previous issue where multiple instances of Loader was discussed and here's where we discussed subclassing. Would be really disappointing to roll back on this.

[dherman]

@matthewp Yes, thanks for the reminder and sorry for the brain fart.

@erights If I'm not mistaken, based on the previous discussion, it should be fine for the Loader constructor to live under Reflect. In particular, it starts with an empty registry so it has no access to additional libraries unless you explicitly grant them. And while it is tied to the realm, this doesn't give it any more authority than the realm already has. Binding to a realm serves to (a) give the created loader the ability to evaluate in that realm, (b) bind modules created in that loader (and consequently functions defined in those modules) to the realm, and (c) bind global variables to that realm's global object.

Do you agree that is a legitimate non-privilege-escalating (unsure if I'm using the terminology right) API to live in Reflect?

[erights]

@dherman I think so. Earlier I wrote:

If the Loader constructor is authority-free, then I have no objection to having it on Reflect. I might
even prefer it. AFAICT, for it to be authority free, all four traps would need to be provided in order for
it to make a Loader instance. Does that seem right?

Does it? Are we talking about the same thing?

(Privilege or authority granting or providing or even bearing are good phrases for what you mean. Escalating or amplification is different.)

[dherman]

Does it? Are we talking about the same thing?

Yes, all four traps have no default behavior, or more likely some of them default to pass-throughs. But the crucial point is that they provide no default access to anything from any other loader.

(Privilege or authority granting or providing or even bearing are good phrases for what you mean. Escalating or amplification is different.)

Thanks :)

[caridy]

Ok, it seems that we are in agreement, I will go ahead and make the corresponding changes.

[matthewp]

What is the System.registry in @dherman's comment? If it's the loader's registry why is it a separate object and not on the loader instance itself?

[caridy]

@matthewp the way I see it, all loader objects bound to the same realm should share the registry (and that includes the default loader per realm). Loader instances will not provide isolation, realms will do that, and this will prevent fetching, parsing and executing the same module multiple times in the same realm. Such as step is not in the spec just yet, nor the creation of the registry.

[dherman]

@caridy We came to the opposite conclusion in issue #7 -- the Loader class is there to allow you to create multiple distinct registries within the same realm.

@matthewp Yeah, strike System.registry from that list; that's just part of what System.loader encapsulates.

[caridy]

Ok, in that case I will clarify that loader.[[Registry]] should be created during instantiation.

[matthewp]

Exposing the registry in some way (no opinion as to how) is still probably needed to solve #17

[dherman]

@matthewp Yes, for sure. The details of the registry reflection API are blocked on the sync loading questions raised by Node, but iterating definitely has to be there.

[caridy]

Solved by #65

[caridy]

• Reflect.Loader is the base class for loader objects.
• System.loader is the default instance of loader per realm, usually an instance of a subclass of Reflect.Loader (depending on the runtime, browser implementation might be slightly different than node).

[domenic]

Why is Loader in Reflect? It is a significant departure from what is currently in Reflect. Can't it be in System?

[caridy]

@domenic Loader Base Class is under Reflect (more details here: #34 (comment)), while default loader instance is on System.

[matthewrobb]

I don't want to add noise to this discussion but just curious why you can't put Loader in the default loaders registry?

System.loader.import("@Loader")

[caridy]

update: we will be lifting Reflect.Loader and Reflect.Module back to ecma, I plan to carry on with this soon, which means this spec will no longer add things to Reflect, only System and internal slots.