Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflect is not an appropriate place for Loader #76

Open
domenic opened this issue Aug 21, 2015 · 24 comments
Open

Reflect is not an appropriate place for Loader #76

domenic opened this issue Aug 21, 2015 · 24 comments

Comments

@domenic
Copy link
Member

domenic commented Aug 21, 2015

I don't think it's generally appropriate, or at least not good practice, for web specs (like whatwg/loader) to modify ES built-ins (like Reflect).

I also am generally against putting anything on Reflect that is not a JS representation of an object's internal method. Right now that is all that is there. Treating Reflect as a general dumping ground for stuff that feels vaguely related to reflection is wrong. And the loader isn't even related to reflection anyway, except by very stretched definitions.

System.Loader seems like a fine place to put the base system module loader class. Call it System.BaseLoader if you want.

@matthewrobb
Copy link

My suggestion to @caridy was to expose it AS A MODULE since it's existence necessitates there being a public facing API for importing it.

@matthewrobb
Copy link

Admittedly I can see that suggestion as being a no-go due to the can of worms discussions it would likely lead to around built in modules.

@dherman
Copy link

dherman commented Aug 21, 2015

@domenic Gonna think about this for a few minutes but one question: do you feel Reflect.Module is appropriate?

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

No; sorry I didn't mention that. But yeah, same arguments:

  • Again, whatwg/loader should not modify ES built-ins.
  • Module is not an internal method.
  • And although it's called a "Reflective module record," it's not about reflection of the object model in any traditional sense I can understand, like Reflect.ownKeys or Reflect.isExtensible or even Reflect.call.

@dherman
Copy link

dherman commented Aug 21, 2015

@domenic Where would you propose the Module constructor live?

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

I don't really understand the scope of System, but it seems fine. Alternately the global seems fine. If there is concern it is too generic, then maybe it is called ReflectiveModule?

@wycats
Copy link

wycats commented Aug 21, 2015

Again, whatwg/loader should not modify ES built-ins.

Can you explain why window is special here?

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

Because it is the global object.

@wycats
Copy link

wycats commented Aug 21, 2015

@domenic according to your philosophy, there should be a window.web or something that the rest of the web specs should go under. In principle, do you think that's true?

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

What? How does that follow from anything I've said?

The global object is where host environments define their properties and extensions. See the ES spec:

In addition to the properties defined in this specification the global object may have additional host defined properties.

@wycats
Copy link

wycats commented Aug 21, 2015

A conforming implementation of ECMAScript may provide additional types, values, objects, properties, and functions beyond those described in this specification. In particular, a conforming implementation of ECMAScript may provide properties not described in this specification, and values for those properties, for objects that are described in this specification.

@wycats
Copy link

wycats commented Aug 21, 2015

@domenic I'm not trying to troll. I just want to understand why you think that web specs should never extend built-ins.

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

Your quote is to allow ES implementations to add non-standard properties. It's not for the host environment, nor is it for standards.

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

Let's put it this way. I don't think web specs should go add Promise.prototype.cancel. Similarly I don't think web specs should go add Reflect.Whatever.

@wycats
Copy link

wycats commented Aug 21, 2015

@domenic Ok, I think I got you now. 😄

You don't want the DOM to short-circuit discussions happening in TC39, with possible negative consequences if the extensions the DOM makes don't match (or cause confusion with) future versions of JS.

Am I getting it now?

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

Pretty much! Stated in those terms, in this case what we have is a web spec deciding unilaterally that Reflect is a place to put things that seem vaguely reflective, whereas TC39 has to-date flirted with that idea in mailing lists, but so far only put internal methods there.

@dherman
Copy link

dherman commented Aug 21, 2015

@domenic On first reading I missed what you said about WHATWG modifying builtins. I don't have a fully-formed opinion yet about where Loader and Module belong, but I want to address that point.

Loader involves a lot of cross-cutting interplay between the browser loading process, the extensibility mechanisms, and the core semantics of modules. This spec is serving as the frontier of work figuring out that interplay. As we go along, we're learning where the natural boundaries lie between generic ECMAScript behavior and browser-specific behavior.

So while this work is happening in this repo, some of it should get uplifted back to ES and eventually removed from this spec. While it'd be nice to know in advance which stuff belongs in the web space, and which stuff belongs in this spec, we're learning where the boundaries lie as we go.

I was talking with @annevk the other day -- recall he wrote the canonical "don't monkey-patch specs" blog post -- and he agreed that monkey-patching is actually a good way to describe initial proposals before proposing them for uplift in their proper spec.

TL;DR because of cross-cutting concerns, it makes sense to do the work in one place and make sure the design is coherent, then ultimately take some of the material back to TC39 for absorption back in ECMAScript. But we shouldn't let the fact that it's happening here dictate API categorization.

@dherman
Copy link

dherman commented Aug 21, 2015

what we have is a web spec deciding unilaterally that Reflect is a place to put things

I never have the intention of doing anything unilaterally here. But I think concretely, what we should make sure to do is flag the stuff that affects ECMAScript proper and bring it to the attention of TC39. Why don't I schedule a topic at the next face-to-face to discuss the status of the Loader work and specifically highlight topics like this one?

@domenic
Copy link
Member Author

domenic commented Aug 21, 2015

That sounds great, yeah :).

@caridy
Copy link
Contributor

caridy commented Sep 27, 2015

@domenic @dherman @wycats I did some preliminary work on https://rawgit.com/caridy/ecmascript-loader/master/index.html (as discussed last week) to move Reflect.Loader and Reflect.Module back into 262 since they are authority free. Here are my notes:

  • it seems that a good portion of the loader spec is in fact authority free.
  • abstract operations and registry class were needed by Reflect.Loader and Reflect.Module (those were moved as well under the assumption that they are "authority free" as well.
  • Reflect.Loader.<symbols> is up for discussion (we can certainly move them if needed).
  • there are at least 4 algos that are still pending to be implemented (mostly abstract operations for Modules)

Let's try to get to consensus about this soon, so we can clean up the loader spec and focus on the missing features for milestone 0.

@domenic
Copy link
Member Author

domenic commented Sep 27, 2015

I still disagree with putting these on Reflect. Regardless of what spec they live in, my second paragraph from the OP stands.

If they are supposed to live in 262, a new global seems best.

Sorry that I wasn't there on the last day to be part of these discussions.

@dherman
Copy link

dherman commented Sep 29, 2015

@domenic

Regardless of what spec they live in, my second paragraph from the OP stands.

OK, so let's attack that then. :) I disagree! The idea that Reflect is only for reflection of plain objects is not an overt constraint I'd ever heard of. A constructor for reflectively constructing exotic module objects is not "vaguely reflective," it's just... reflective. Meanwhile, the library is not called ObjectReflection or PlainObjectReflection, it's just called Reflect. The chapter of the ES6 it lives under is called "Reflection."

Put differently, you use pejoratives like "vaguely" and "dumping ground" to impute motives -- that I was careless and didn't really know where this stuff belonged. To the contrary, I deliberately chose Reflect because it is exactly what I understood the Reflect namespace to be about: operations that expose the underlying data model to user control. (If I had it to do over, I'd put Proxy in Reflect as well.)

If they are supposed to live in 262, a new global seems best.

This is a strange logical implication. Earlier you were saying if they don't live in 262 they shouldn't be in Reflect. But the bureaucratic tail should not wag the design dog. What standards body we work in is not the point. (In fact I do think we should go ahead and move these parts of the spec into TC39 proposals soon, and in fact last week we talked about doing this.)

I think the real point of contention here is about the granularity of standard library "modules." Right now, since things live on the global, there's a strong incentive towards small numbers of globals and consequently large modules. You and I both agree that the ideal is closer to substackianism. The question is really just one of our moment in time: we don't yet have a set of conventions for standard library naming and organization, or for how they interact with naming conventions in the ecosystem at large.

Now, last week, @stefanpenner and @wycats said they're working on this and believe we can make some good headway. So ideally we can get to a point where we stop having these debates about how best to arrange deck chairs on the titanic window object. And we aren't immediately blocked on the naming of this module since it's not required for the first milestone of the roadmap.

But my opinion is that if we do get blocked:

  • it's fine to put the necessary reflection API's in globals
  • we can discuss the naming in TC39 instead of here if you want
  • if they live in globals, we should respect the naming and organizational constraints of globals, which are different from the naming and organizational constraints of modules
  • Reflect is a fine place for reflection API's

@domenic
Copy link
Member Author

domenic commented Sep 29, 2015

OK, so let's attack that then. :) I disagree!

Cool, let's go at it! :)

The idea that Reflect is only for reflection of plain objects is not an overt constraint I'd ever heard of.

Right, but it is one that Reflect currently follows, and I personally would like to hold the line there. (I'm not committed to it, but it seems nice.)

A constructor for reflectively constructing exotic module objects is not "vaguely reflective," it's just... reflective.

Here's what I mean: why don't we put the Object and Array constructors there? They allow you to reflectively construct objects and exotic arrays! Why not Proxy? Why not anything? "Reflectively constructing" is more or less contentless, compared to just "constructing."

The chapter of the ES6 it lives under is called "Reflection."

I'd like to see a stronger argument why all of this stuff is conceptually reflective, that doesn't just reduce to making the term reflective meaningless. Some links to definitions would probably help me. You might be right, but so far my experience with "reflection" in ES and in .NET does not include this realm of stuff (loading configuration or first-class namespace objects) at all.

I think the real point of contention here is about the granularity of standard library "modules." Right now, since things live on the global, there's a strong incentive towards small numbers of globals and consequently large modules. You and I both agree that the ideal is closer to substackianism.

I think there might be a disconnect here. As @littledan has mentioned a number of times, there's actually zero virtue in minimizing the number of globals. There's 730+ global constructors already. We're not going to stop this pattern. ES should embrace the patterns of the ecosystem it lives in, and not try to forge its own where it hides under namespace objects. So I think a couple more globals for module-related stuff seems fine.

we can discuss the naming in TC39 instead of here if you want

To this and above comments about bureacracy, I really don't care about venue. The point I was trying to make earlier was about the separation between web specs and ES conceptually, not about who defines things. To me the loader has always been a "web spec": it's not suitable for all environments, etc. So from that perspective it shouldn't be messing with ES globals. Similar to how we say that Node environments shouldn't go adding things to Math or JSON or Reflect, but instead should use their own globals (Buffer or process or whatever).

Maybe your thinking has evolved so that you think parts of the loader work are universally applicable to all environments, in which case that portion of my argument doesn't apply. But it was never about venue :)

@dherman
Copy link

dherman commented Sep 29, 2015

"Reflectively constructing" is more or less contentless, compared to just "constructing."

What makes it reflective is not that it's a runtime operation, but that it constructs something that interacts in non-trivial ways with elements of the base level semantics. The module system does not allow you to use arbitrary objects as modules. You can only use Module objects (e.g. to register in the registry, or to produce as the result of loading hooks), and those collaborate with the base semantics of import and export to make the system work.

Similarly, the Loader class collaborates with a realm and a module registry so that when the base semantics of ECMAScript processes import declarations associated with it, its hooks get triggered.

In short: the loader API is the hooking system by which user code can intercept and modify the behavior of the import syntax.

Some links to definitions would probably help me.

From Wikipedia:

In computer science, reflection is the ability of a computer program to examine...and modify its own structure and behavior (specifically the values, meta-data, properties and functions) at runtime.

That's exactly what Module and Loader allow you to do. As another example, in the original Java paper on class loaders (which was an inspiration behind the Loader API), Liang and Bracha wrote:

Class loaders can be thought of as a reflective hook into the system's loading mechanism. Reflective systems in other object-oriented languages...have provided users the opportunity to modify various aspects of system behavior. One could use such mechanisms to provide user-extensible class loading; however, we are not aware of any such experiments.

I'm happy to entertain other organization of the standard library, but I'm just pretty bewildered that you think it's a stretch to call a dynamically user-hookable module system reflective.

As @littledan has mentioned a number of times, there's actually zero virtue in minimizing the number of globals. There's 730+ global constructors already.

That's at the very least an exaggeration. You're eliding a serious difference of scale. Nobody would be bragging if npm had 730 packages. We wouldn't splat all the SIMD constructors onto the window object (as opposed to under a single top-level SIMD object).

That said, I wouldn't lose sleep over there being a grouping for module-related reflection, whether under a global Module.* grouping or Reflect.Module.*. I'm not sure if a global with a generic name like Module causes compatibility risks but maybe not?

To me the loader has always been a "web spec": it's not suitable for all environments, etc. So from that perspective it shouldn't be messing with ES globals. ... Maybe your thinking has evolved so that you think parts of the loader work are universally applicable to all environments, in which case that portion of my argument doesn't apply.

This is really important to be clear about: from the beginning, this spec has contained a mix of web-specific stuff and stuff that is universal to ECMAScript. Put differently, TC39 didn't make a surgical cut when it moved stuff out from ES6 into WHATWG. In particular, the Module and Loader constructors have nothing whatsoever web-specific about them. They're core, necessary parts of the ECMAScript semantics.

So we either have to allow for this spec to contain some elements that are universal to ECMAScript, or we have to move those parts back to TC39. To be clear, I am happy to move those parts back to TC39, and I plan to. I never had any intention of circumventing TC39 (I'm a member after all!). It was simply that TC39 decided to move that stuff to WHATWG, so that's what I did. But in retrospect, things like Module and Loader, as well as some of the missing syntactic constructs (like the metadata-import syntax) should never have been moved to WHATWG and should have simply been deferred to micro-proposals in the post-ES6 process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants