whatwg / loader

Loader Standard
https://whatwg.github.io/loader/
Creative Commons Zero v1.0 Universal
607 stars 45 forks source link

reflection: imperative equivalence for declarative import #36

Open caridy opened 9 years ago

caridy commented 9 years ago

as today, we only have loader.import('./foo.js').then(m => {...}), which is reflective to import * as m from "./foo.js", as we discussed earlier today, we might need to provide reflection for the other two options (default and named exports import declarations).

@dherman suggested:

local.import("./foo.js").then((def, named) => ...)
local.importNamespace("./foo.js").then(m => ...)
domenic commented 9 years ago

I really like the (def, named) => signature.

What is different between local.importNamespace("./foo.js").then(m => use(m)) vs. local.import((def, named) => use(named))?

johnjbarton commented 9 years ago

(Sorry for the meta discussion, but where I'm from "reflection" means functions for analysis, http://en.wikipedia.org/wiki/Reflection_%28computer_programming%29.

This subject seems to be "Imperative equivalence for declarative loading" or similar).

caridy commented 9 years ago

To clarify, or to raise the question, depending on how you look at this, but local.import("./foo.js").then(def => ...) is really not reflective as it is the current proposal, essentially because def will not get updated if the value change in the provider module.

domenic commented 9 years ago

Hmm maybe that is an argument for only having the namespace form.

guybedford commented 9 years ago

importDefault?

arv commented 9 years ago

I'm not sure what problem we are trying to solve here?

How is

local.import("./foo.js").then((def, named) => ...)

better than:

local.import("./foo.js").then({default: def} => ...)
local.import("./foo.js").then({foo} => ...)

Lets try to keep the API surface area minimal. The special cases can always be done in user code and added later version of the spec.

matthewp commented 9 years ago

Yes, I'm with @arv on this, es6 with make this elegant enough without the need for multiple methods/signatures. loader.import should return the module. import/export is already complicated enough (I still have to reference this all the time), let's not muddle the dynamic api as well.

dherman commented 9 years ago

@arv @matthewp A low-level reflection API can be minimal and general. But the local import object is a high level API, meant for common use. And it's meant to be the dynamic analog of what you can do with the declarative syntax. It should have its defaults be symmetric with the surface syntax.

In the surface syntax, when you import a default, you don't have to write d-e-f-a-u-l-t. It's just the simplest thing where you have to say the least. In short, we want to allow:

local.import("./my-app-controller.js").then(MyAppController => ...)

as the most convenient case just like

import MyAppController from "./my-app-controller.js";

would be.

Another way of looking at this is that single-export needs to be as convenient in this API as it is today in existing JS module systems. Not having to say default explicitly is very important to the ergonomics.

But we also have to allow importing the namespace object, of course. And sometimes a module will not have a default export, so forcing you to write .then((dummy, named) => ...) is obnoxious for those cases. Hence a second method specifically for asking for the namespace object.

dherman commented 9 years ago

@caridy @domenic It's important to understand the use case of local import as not being about reflection but rather about dynamic loading. Reflection is a low-level operation meant for when you're trying to extend the base semantics of the language/system. Dynamic loading is simply a thing apps commonly need to do.

Also keep in mind that .import will not resolve the promise until the module has been initialized, fully executing its top-level code. The overwhelming majority of the time, the aliasing behavior is only important for the initialization phase where top-level bindings are being set up. It's extremely rare to mutate top-level exports after that point. So it's inappropriate to optimize this API around that as a constraint.

Again, the low-level loader API should fully generalize the semantics, and there it is certainly sensible to have operations that just produce the namespace object as the most general thing. But the purpose of the local import API is as a high-level API whose ergonomics and style should mirror the ergonomics and style of the declarative syntax.

caridy commented 9 years ago

Not having to say default explicitly is very important to the ergonomics.

Yes

It's extremely rare to mutate top-level exports after that point.

Agree. We can probably make the case that the value of the default export should never change, in which case something like local.import("./foo.js").then(def => ...) should be fine.

matthewp commented 9 years ago

You have to say default all the time already, it's a common way to export things. I see your point about having symmetry between the forms. But the declarative form is nice because of syntactical affordances you won't have here.

Question,

local.import("./foo.js").then((def, named) => ...)

even valid? I thought Promise only had a single return value.

dherman commented 9 years ago

You have to say default all the time already, it's a common way to export things.

The important distinction is that you only say default inside the implementation of a module, just as you have to do something explicit in CommonJS/Node (module.exports = ...;). But on the consumer side, you don't have to think about it having a name. Anyway sorry to beat a dead horse; it sounds like you agree the symmetry is important.

I thought Promise only had a single return value.

LOLOLOL. (At least I'm slightly comforted by the fact that the lead of the promises spec didn't catch this either. ;-P)

I think a reasonable alternative is two forms, with the short name providing the default, and the longer name providing the namespace object:

l.import("./rubber-chicken.js").then(RubberChicken => ...)
l.importAll("./utils.js").then(utils => ...)

The latter lets you extract the default export if you want:

l.importAll("jQuery").then(namespace => {
  let $ = namespace.default;
  ...
});

Or you can combine the two operations:

Promise.all(l.import("jQuery"), l.importAll("jQuery"))
  .then(([$, namespace]) => ...);
caridy commented 9 years ago

jejeje, importAll resonates well with me considering the work we have done on https://github.com/estree/estree/blob/master/es6.md#exportalldeclaration, which is symmetrical to the import all concept. I like it.

still we should clarify whether and how people will be able to update the default export from within a module, if there is no way to do that, then this syntax will be perfect.

johnjbarton commented 9 years ago

I think a reasonable alternative is two forms, with the short name providing the default, and the longer name providing the namespace object:

l.import("./rubber-chicken.js").then(RubberChicken => ...)
l.importAll("./utils.js").then(utils => ...)

I would appreciate a simplified explanation of what you all are talking about here. What is a "namespace object"?

dherman commented 9 years ago

What is a "namespace object"?

Also known as a module instance object in earlier versions of the module spec; ES6 now calls them "module namespace exotic objects." For example when you do

import * as fs from "fs";

It's the object that fs is bound to. It exposes all the module's named exports as properties (and if there's a default export, it has a named property called default that points to that).

johnjbarton commented 9 years ago

Thanks!

So the idea here is that

l.import("./rubber-chicken.js").then(RubberChicken => ...)
l.importAll("./utils.js").then(utils => ...)

is better than

l.import("./rubber-chicken.js").then(RubberChicken => ...);
l.import("./utils.js").then(ns => ns.default).then(utils => ...);

(where my import returns the namespace)?

IMO import should give the namespace. If we really need a thing for default (which we've almost never used in our es6 code), then it should have the special name, eg something obvious like importDefault. The short name should do the non-special thing.

dherman commented 9 years ago

An overloaded API like

l.import("./rubber-chicken.js").then(RubberChicken => ...);
l.import("./utils.js").then(ns => ns.default).then(utils => ...);

is bad because it changes what it returns based on whether you have a default export. But adding a default export should be a backwards-compatible API evolution: it should not break clients who aren't currently using it. The module system was designed with this principle as well.

IMO import should give the namespace. If we really need a thing for default (which we've almost never used in our es6 code), then it should have the special name, eg something obvious like importDefault. The short name should do the non-special thing.

I recognize there will be different styles in different communities, but default export is a highly popular pattern in many many JS ecosystems (from jQuery and underscore to AMD to NPM), and the ES6 module system picked that as a winner. In particular, the ergonomics of the import syntax in the module system are optimized for default export. The ES6 design isn't going to change, and the design of the reflective API has to parallel the design of the syntax.

johnjbarton commented 9 years ago

An overloaded API like

l.import("./rubber-chicken.js").then(RubberChicken => ...);
l.import("./utils.js").then(ns => ns.default).then(utils => ...);

There is no overloading here. The namespace is returned, always.

default export is a highly popular pattern in many many JS ecosystems (from jQuery and underscore to AMD to NPM), and the ES6 module system picked that as a winner.

That's quite different from my read. To me this default thing was added to appease the backward compat crowd. Really it seems obvious: if the ES6 module system picked it as winner then we would not need the funky extra default keyword at all!

Our dynamic import should mirror our static import:

  Reflect.loader.import('./foo.js').then(foo => ...);  // namespace foo

mirrors

  import * as foo from "./foo.js";

and

  Reflect.loader.importDefault('./foo.js').then(foo => ...); // default from foo

mirrors

  import { default as foo } from './foo.js';
domenic commented 9 years ago

To counterbalance @johnjbarton, in my ES6 projects I almost never use named imports.

I am still not very comfortable with the fact that .import() gives a non-updatable binding while import gives the magic-updating one. I would be more comfortable if we forced people to type .default since then the exotic object [[Get]] would be triggered each time, ensuring the binding stays updated.

johnjbarton commented 9 years ago

@domenic Could you point to an example? (Mine is the Traceur source https://github.com/google/traceur-compiler).

Is there a cogent pro/con discussion on this issue?

caridy commented 9 years ago

@johnjbarton nobody will do import { default as foo } from './foo.js';, they will do import foo from './foo.js'; that's the whole point @dherman is trying to make, you should NOT have to use the keyword default to import the default export, and that should be the most common use-case.

caridy commented 9 years ago

To counterbalance @johnjbarton, in my ES6 projects I almost never use named imports.

same here, you can check the of the packages used in formatjs.io, we use named exports in some places, but the majority of them are default exports.

ericf commented 9 years ago

While I do like the looks of the API that @dherman is proposing being the dynamic form of the importing the default export, I want to echo @domenic's concern about the non-updating binding. To me, this difference is trumping the economics argument and I think local.import() should return a Promise for the "module namespace exotic object."

My gut feeling is that the number of dynamic imports within an app's code base will be dwarfed by those which use the declarative syntax, so I don't think the economics arguments holds up against the wtf-js one.

matthewp commented 9 years ago

I'm still slightly on the side of only having the one signature import that returns the module. I do not see the benefit of only getting part of a module.

Regardless please do not choose importAll if you go this route. Users will assume it takes an array of modules to import. "import all of these things" makes sense, "return all of the bindings from this import" less so.

Personally I'm with @johnjbarton and use mostly named imports. But why should we base this on conjecture? It seems like there is enough es6 code floating around that a decent survey could be done.

matthewp commented 9 years ago

I think we might be over-estimating how used import is going to be. In my experience we use it in 2 or 3 places primarily:

  1. To initially load your application. This will be superseded by <module>.
  2. To progressively load parts of an application.
  3. To conditionally load modules (based on environment variables for example).

1 is going to go away, and 2 and 3 are not used by a lot of applications anyways, and when they are it is used sparingly. I think import is safely within the realm of sophisticated applications. People working on these applications can handle default, imo.

johnjbarton commented 9 years ago

@caridy Nevertheless, any explanation of import foo from './foo.js'; will have to include the word default. To distinguish dynamic import of the namespace (that is, default thing we get without the default keyword) from the dynamic import of the developer-designated default we need a modifier. The natural modifier is 'Default', applied to obtain the developer-designated default. Applying a modifier "All" and having it return the thing we get by default without using default just compounds the confusion, in much the same way as I have done here.

dherman commented 9 years ago

Yikes, I believe I've perpetrated a pretty big confusion here. (Mea culpa!) There are two separate APIs to discuss, and I think at various points in this thread we may have been talking about one or the other. For clarity, let me give them names:

The difference

My view is that a reflective layer is meant more for advanced, frameworky code that is thinking at the level of the mechanics of the language semantics, but async import is more common. @matthewp fairly makes the point that it's still pretty rare. As @ericf put it in a private chat, it'll be common per-app-codebase but rare in frequency; most apps will have it, but each app will typically have only a handful of instances of it.

Constraints

Reflecting export mutability

We all agree it's a requirement for the low-level reflective API to reflect the mutability of exports. Module namespace objects are live views of the module's exports, so in that sense it's reasonable for the low-level reflective API to just have a single method that produces the namespace object.

Respecting the single-export mental model

The design of the module system allows a mental model where you can conflate the concept of a module's default export with the module itself. For example, you can create a UI component called AdminPanel that is the single export of a module, and allow the consumer to think of "admin panel" as being both the module and its sole export:

import AdminPanel from "./admin-panel.js";
...
someComponent.click()
  .then(() => {
    let panel = new AdminPanel();
    ...
  });

Notice how both the name of the module and the name of its exported abstraction are the same concept.

Now, if the programmer decides the admin panel is used rarely, they can do progressive loading of the code and change this to an async import. If we provide a convenient method for dynamic loading of default export, the programmer writes:

import local from this;
...
someComponent.click()
  .then(() => local.import("./admin-panel.js"))
  .then(AdminPanel => {
    let panel = new AdminPanel();
    ...
  });

If we only have a method that produces a namespace object, they have to drop below that level of abstraction:

import local from this;
...
someComponent.click()
  .then(() => local.import("./admin-panel.js"))
  .then(({ AdminPanel: default }) => {
    let panel = new AdminPanel();
    ...
  });

IOW, instead of the local import being just an async way of importing, it's also a reflection mechanism: you have to think about the details of the JS module system instead of just thinking about your app concepts.

Hazard of late mutations of default export

If the admin panel decides to mutate its default export late at runtime, it's possible for uses of a default-export-only API to go stale. For example, the admin panel might do something like:

var AdminPanel = class { ... };
...
// mutate the default export at runtime
someComponent.click().then(() => {
  AdminPanel = ...;
});
...
export { AdminPanel as default };

and then code that uses it might hang onto a stale binding:

...
local.import("./admin-panel.js")
  .then(AdminPanel => {
    someOtherComponent.click().then(() => {
      let panel = new AdminPanel(); // oops! didn't get the updated binding
    });
  });

Note that this is not an issue for mutation of the default export during initialization, since the import promise is only fulfilled after initialization completes. So for example you won't witness incomplete initialization of cycles. They have to explicitly modify the binding later on.

Consistency between the two layers

If both loader and local objects have importing APIs, for them to be subtly different would be confusing. For example, if loader.import() produces a namespace object but local.import() produces the default export, this would be an inconsistency between two APIs that are highly related.

My conclusion

I think this comes down to how real we think the risk of the hazard is and how bad the mental model violation is. IMO, the risk is low because people rarely mutate their exports, and the mental model violation is very bad for a high-level API, because the whole purpose of the design of the module system was to provide syntactic sugar to empower the conflation of single exports with their module. All that said, I recognize @matthewp's point that this is not going to be a ton of code per codebase, because progressive loading of an app is typically only done in a few select places.

So my preference is that we support both .import and .importAll on both objects. This allows you to get at the full namespace object and to get the full live-updated version of the default export if you have to. The worst-case scenario is that some popular library or libraries to export a runtime-mutating default export and it becomes a best practice to avoid .import and use .importAll, but I think it's better as a best practice to say don't mutate your exports after module initialization.

update: cleaned up some of the promise code in the examples

caridy commented 9 years ago

+1 on *.import() and *.importAll() where:

matthewp commented 9 years ago

Fair points, I won't fight this too hard even though I don't love it.

Now back to bikeshedding :-) I really think All is wrong here. If a function is a verb and its arguments the object, importAll says "import all of these modules". But what we really are saying is "import everything from this module". importFrom maybe? If its named importAll I guarantee you devs will confusingly do:

importAll(['./login-panel.js', 'admin-panel.js']).then(function(panels) { });
dherman commented 9 years ago

Good point, but both forms correspond to import _______ from "..."; so it's not the right distinction. I hate to tax both APIs but what would you think of

Does that clear up the ambiguity with importing multiple modules? I'm not sure what I think of it...

dherman commented 9 years ago

A more concise naming scheme that I'm warming to would be import and importStar. The correspondence to the analogous syntax is very obvious that way.

caridy commented 9 years ago

again, in estree we were from *Batch, to *Star to *All. I think we can settle with importFrom() and importAllFrom(), it is pretty clear for me that it does not import multiple modules, if that is the case, then it will be importFromAll().

arv commented 9 years ago

How about import and importModule? On Mar 14, 2015 7:40 AM, "Caridy Patiño" notifications@github.com wrote:

again, in estree we were from Batch, to Star to *All. I think we can settle with importFrom() and importAllFrom(), it is pretty clear for me that it does not import multiple modules, if that is the case, then it will be importFromAll().

— Reply to this email directly or view it on GitHub https://github.com/whatwg/loader/issues/36#issuecomment-80480872.

arv commented 9 years ago

How about import and importModule?

matthewp commented 9 years ago

I'd hate to tax import even for the sake of consistency. I don't hate the import and importStar combo.

MajorBreakfast commented 9 years ago

I think importAll is too close to Promise.all. I'm for importModule or importNamespace.

However I can't decide which variant is better:

@dherman Your example above has a problem: Can you correct that?

local.import("./admin-panel.js")
  .then(AdminPanel => someOtherComponent.click())
  .then() => {
    let panel = new AdminPanel(); // <- AdminPanel is undefined
  });
dherman commented 9 years ago

@MajorBreakfast Fixed, thanks. Having the general method get the shorter name is the wrong way around. "Make the common cases easy and the uncommon cases possible."

dherman commented 9 years ago

Ah, there's one I overlooked: the namespace form is import * as ns from "foo", so the as could be the differentiator: import vs importAs. That's actually pretty sweet. Short and sweet for both, privileges single-export, connects to the syntax. I like it!

brettz9 commented 9 years ago

If it does not already, I would imagine that import-as form could be modified to allow only partial imports such as import { newPropName1, newPropName2 } as ns from "foo"; (i.e., producing ns === {newPropName1: ... newPropName2: ...}). If this form is or could be allowable, then the meaning would thus seem to be implied more by the * than the as.

What about importFull?

And for either case, how about actually allowing the syntax as mentioned in https://github.com/whatwg/loader/issues/36#issuecomment-79407109 for import (as opposed to the proposed importAs). Together with parameter destructuring, this would make writing multiple imports very elegant.

dherman commented 9 years ago

producing ns === {newPropName1: ... newPropName2: ...}

I don't think this is necessary; the idea of partial imports is not that only a partial module is imported but simply that you're only binding the parts you care about in local variables. Destructuring gives you the analogous thing:

local.importAs("admin-panel").then(({ foo, bar }) => { ... });

I don't see much benefit to allocating a new object with only some of the exports defined on it, and it also brings in new complexities like whether to include other state that might live on the namespace object (like expando properties with Symbol names).

And for either case, how about actually allowing the syntax as mentioned...

You get that with Promise.all already, but maybe you'd like to see this available with more convenience? ISTM a convenient way to do this is to overload import and importAs to accept either an array or a stringable, so if you write:

local.import(["foo", "bar", "baz"]).then(([foo, bar, baz]) => { ... });

you wait for all three, and if you write:

local.import("foo").then(foo => { ... })

you only wait for the one. Otherwise we start getting a combinatorial explosion of awkwardly named methods (import, importAs, importAll, importAsAll).

But I can see how the overloading would be more convenient than

Promise.all(["foo", "bar", "baz"].map(s => local.import(s))).then(([foo, bar, baz]) = { ... });
caridy commented 9 years ago

:+1: on importAs

matthewp commented 9 years ago

The number of choices in this discussion is evidence to me that trying to match the static API is not a good way to go. They just aren't alike.

Maybe we need to just admit that ES6 doesn't have a single export and not tailor the API to the hack to make it (almost) appear that it does. Dynamic importing is an "advanced" API anyways, I think I'm back on board for having a single loader.import that loads the entire module and then using destructors to get the default, if that's what you need.

brettz9 commented 9 years ago

@dherman +1 for overloading with arrays for the added convenience.

MajorBreakfast commented 9 years ago

I don't think that the name importAs fits:

//Actual renaming: "as" fits
import * as name from 'module'
import { hello as world } from 'module'
// No renaming: "as" doesn't fit
System.importAs('module')
  .then(module => { ... })

I think I'm a bit @matthewp now (just import) or import+importDefault. Those two options are the only options that have descriptive names. The other feel wrong to me.

+1 for overloading with arrays.

@dherman Found another mistake: .then(({ AdminPanel: default }) => { should be .then(({ default: AdminPanel }) => {, It's propertyName: variableName = defaultValue (mdn)

domenic commented 9 years ago

-1 for overloading with arrays. We have compositional primitives for a reason. People can write their own dwimImport function if they really can't deal with Promise.all.

caridy commented 9 years ago

-1 as well on the overloading.

brettz9 commented 9 years ago

In my view, a person should be able to write declarative code out of the box. There is almost nothing more basic in JavaScript than wishing to load modules, and having to copy in extra boilerplate just to do that detracts, imo, from its elegance, quick digestability to others reading the code, and speed of development. One can hide away implementation details within dedicated modules but the actual loading should not need implementation details, imv.

matthewp commented 9 years ago

@brettz9 It's hard to debate this until https://github.com/whatwg/loader/issues/27 is answered. From my perspective the spec authors haven't decided yet if they want Loader to be low-level and follow Extensible Web (which would be my preference) or be a high-level API that accomplishes only what is needed to load ES6 modules. From the issues I've participated in sometimes it seems they want it to be the former and sometimes the latter.

We've seen in the past what happens when an API is both too low and too high (IndexedDB) so I hope this is decided quickly; it will make these other discussions much easier.

johnjbarton commented 9 years ago

Earlier @dherman said:

Yikes, I believe I've perpetrated a pretty big confusion here. (Mea culpa!) There are two separate APIs to discuss, and I think at various points in this thread we may have been talking about one or the other. For clarity, let me give them names:

  • reflective import: an API on Reflect.Loader.prototype that provides a complete reflection of what the module system can do
  • async import: an API on the object bound via import local from this; that provides a way to progressively, asynchronously import modules

Is the second one a replacement for what we used to call the dynamic loading: eg System.import()? Or is there three APIs using the word import()?

caridy commented 9 years ago

@johnjbarton no. async import reference to the ability to access a loader instance that is bound to a module, and therefore it can perform the exact same import operations that you can achieve using the declarative syntax. We don't know yet, but in previous discussions it seems that it aligns well with what we want to achieve with the global loader, in other words, they will have the same API, although the internals are a little bit different since one of them is relative to a page, the other is relative a the module.