-
Notifications
You must be signed in to change notification settings - Fork 45
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
reflection: imperative equivalence for declarative import #36
Comments
I really like the What is different between |
(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). |
To clarify, or to raise the question, depending on how you look at this, but |
Hmm maybe that is an argument for only having the namespace form. |
|
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. |
@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 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 |
@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 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. |
Yes
Agree. We can probably make the case that the value of the |
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. |
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 (
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]) => ...); |
jejeje, 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. |
I would appreciate a simplified explanation of what you all are talking about here. 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 |
Thanks! So the idea here is that
is better than
(where my import returns the namespace)? IMO |
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.
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 |
There is no overloading here. The namespace is returned, always.
That's quite different from my read. To me this Our dynamic
mirrors
and
mirrors
|
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. |
@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? |
@johnjbarton nobody will do |
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. |
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 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. |
I'm still slightly on the side of only having the one signature Regardless please do not choose 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. |
I think we might be over-estimating how used
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 |
@caridy Nevertheless, any explanation of |
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 differenceMy 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. ConstraintsReflecting export mutabilityWe 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 modelThe 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 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 exportIf 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 layersIf both My conclusionI 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 update: cleaned up some of the promise code in the examples |
+1 on
|
Fair points, I won't fight this too hard even though I don't love it. Now back to bikeshedding :-) I really think importAll(['./login-panel.js', 'admin-panel.js']).then(function(panels) { }); |
@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 |
How about |
I certainly don't feel undecided about it! :) I tried to answer the question in this thread. In short:
@matthewp Async import and reflective import are two different APIs, and they don't have the same design constraints. In particular, async import is tied to syntax so it can never be fully abstractable. This means people are more prone to suffering through the papercuts.
You're eliding the semantic model with the programming model. While the internal semantics of ES6 does not have any special concept of a default export, default export is a central part of the design. It's part of the way modules are thought about and the way they are written. I appreciate people working through the different possibilities here, btw. I know it can be frustrating but IMO it's always good to map out a space of possibilities before landing too soon on a conclusion. In that spirit, another comment coming in a sec... ;-P |
@johnjbarton No, |
So here's another syntactic possibility we hadn't considered previously for async import. :D A late-breaking addition to ES6 was the In that light, a candidate syntax-and-API for async import would be: import.default("Spinner").then(Spinner => { ... });
import.namespace("fs").then(({ readFile, writeFile }) => { ... }); (I can live with making Note that if we specify Promise.all(["Spinner", "Slider", "Gallery"].map(import.default)).then(...);
Promise.all(["fs", "util", "collections"].map(import.namespace)).then(...); |
Forgot to mention, I also propose the metadata (like filename, URL, debugging info, etc) live as a separate subobject, probably |
@dherman Can you explain how this works? Is |
@matthewp It's not reflected as a first-class object, in that you can't get your hands on the object directly. It's like an object where the syntax only allows you to access certain predetermined properties. So for example you can ask for |
But yes, it's contextual per-module. |
That's interesting, I hadn't heard of this feature. I'll have to dive into esdiscuss then, but I think I understand what you're saying. I like the idea overall and agree it's a clever way of working around this problem. You do still want access to the loader within a module, |
I should also show what this ends up looking like in let Spinner = await import.default("Spinner");
let { readFile, writeFile } = await import.namespace("fs");
... |
Very elegant! |
This look pretty elegant! Don't be too frustrated: We can restart the naming discussion around this syntax! :) :) Promise.all(['Spinner', 'Slider', 'Gallery'].map(import)).then(...) // Default import, call import as a function
import.namespace('fs').then(fs => ...) // Namespace import
import('fs', 'readFile').then(readFile => ...) // New idea: Named import shortcut You tell me if treating import as a function does even work. |
Does this need to be a metaproperty and not some form of a function? It seems like a language extension would mean this loader would have to be the backer for all host environments if it assumes it gains meta-properties. |
@bmeck Not having the Loader be the backer for all host environments would be a major mistake. |
@matthewp you need to explain that in depth. loader vs es import/export are 2 different things to my knowledge |
They are different things but users write code that run in multiple JavaScript environments. |
@matthewp it has different implications semantically than existing code/systems (couchdb, webworkers, mongodb, node, ringojs, ...) which is the issue. having the semantics able to match would be nice as well as be much safer for upgrade purposes |
I agree, a breaking change for those legacy systems is probably a no-go. I wish there were more participation here from all interested parties but this repo has been largely dark for the last several months. There's a specification at amodro-lifecycle that attempts to address some of these legacy needs such as Node, I would suggest giving it a read if you have time. |
@matthewp my last experience here did not include metaproperties or requirements for it to be used as the |
The metaproperty proposal only exists in this issue and hasn't been written to any spec AFAIK. There hasn't been any increase in scope to my knowledge, it is now much smaller is scope actually. |
@matthewp we commented on it because it is a language extension which immediately concerned us. A lot of work is often done in conversations and only shows up in specs to replace TODOs etc. at a very late stage if we want to read public available text. I think talking in issues is fairly similar to talking about the spec. |
@matthewp @bmeck just a side note, the |
as today, we only have
loader.import('./foo.js').then(m => {...})
, which is reflective toimport * 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:
The text was updated successfully, but these errors were encountered: