-
-
Notifications
You must be signed in to change notification settings - Fork 65
Asynchronous module.import(path):Promise #50
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| | Title | module.import(path):Promise | | ||
| |--------|-----------------------------| | ||
| | Author | @WebReflection | | ||
| | Status | DRAFT | | ||
| | Date | 2017-01-23 | | ||
|
|
||
| # Asynchronous `module.import(path)` | ||
| There is [a standard effort](https://github.com/tc39/proposal-dynamic-import) | ||
| to bring asynchronous module loading to JavaScript. | ||
|
|
||
| Such effort will take some time before it will be fully agreed, | ||
| and before it will be widely available across all JS engines. | ||
|
|
||
| In the meanwhile, the community could benefit from this _opt-in_ | ||
| proposal, which is 100% backward compatible, as well as future friendly, | ||
| being based entirely on what's been agreed already by TC39. | ||
|
|
||
| ## The pseudo implementation example | ||
| The following code, which works already, | ||
| represents how essential is this additional | ||
| `module.import` method proposal: | ||
| ```js | ||
| // one added method to Module.prototype | ||
| module.constructor.prototype.import = | ||
| function (path) { | ||
| return Promise.resolve() | ||
| .then(() => this.require(path)); | ||
| }; | ||
| ``` | ||
|
|
||
| ### Fully backward compatible | ||
| Every existent NodeJS/CommonJS module can be imported already. | ||
| There is no need to change anything to the existing code-base. | ||
| ```js | ||
| // even core modules work already | ||
| module.import('fs').then(fs => { | ||
| // do something with fs module | ||
| fs.readdir('.', console.log); | ||
| }); | ||
| ``` | ||
|
|
||
| ### Multiple imports too | ||
| Keeping the implementation as simple as possible in order | ||
| to have zero conflicts with current dynamic | ||
| standard `import` proposal, we can use standard | ||
| `Promise.all` method to retrieve at once every needed module. | ||
| ```js | ||
| Promise.all([ | ||
| module.import('fs'), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This proposal should be clear about whether
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the idea is to have a non blocking What the implementation does behind the scene shouldn't be a developer concern: they must be aware the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is intended, then this should be pointed out in the proposal IMO. For example, it should point out that developers must check for race conditions in cases like this: Promise.all([
module.import('a'), // mutate shared external state when imported
module.import('b') // mutate shared external state when imported
module.import('c') // export something depends on the mutated state
]).then(function([a, b, c]) { ... })as this won't happen if
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you give me a concrete example of what kind of race condition you are talking about ?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a does: process.env.foo = 'a';and b does process.env.foo = 'b';and c does: if (process.env.foo === 'a') {
module.exports = { foo: 'b is executed first' };
} else if (process.env.foo === 'b') {
module.exports = { foo: 'a is executed first' };
} else {
module.exports = { foo: 'c is executed first' }
}The export of
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This won't happen if c is a ESM, because ESM must export at the top level.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
once dynamic export default async () => Promise.all([import('a'), import('b')])
.then(([a, b]) => ({
c: x => a(x) + b(x)
}));'cause this proposal idea is to enable asynchronous exports too. However, being the current logic based on synchronous require and the The scheduling is still incremental. Promise.all([
Promise.resolve(1).then(console.log),
Promise.resolve(2).then((v) => {
let t = Date.now();
while (Date.now() - t < 1000);
console.log(v);
}),
Promise.resolve(3).then(console.log)
]);
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What I mean is, is this behavior intentional, or is it just a coincidence due to the polyfill? Is the choice of the blocking and synchronous Another reason this is relavant is that the order of the ESM loader in Node.js is still in discussion: https://docs.google.com/presentation/d/1xZILfv5WeUBKyQ9s1w8zArNgzTMGRFQXyRseSskjcjw/view#slide=id.g1bfb475df0_0_166 If this proposal wants to stay inline with the decision on that side, it would probably create less confusion to future users.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, the reply by email didn't work that well. I am waiting for this #39 (comment) to be decided/agreed to better answer your question. Right now the polyfill as it is grants order. If async, the order won't be granted. |
||
| module.import('path') | ||
| ]).then(([fs, path]) => { | ||
| fs.readdir( | ||
| path.join(__dirname, 'test.js'), | ||
| console.log | ||
| ); | ||
| }); | ||
| ``` | ||
|
|
||
| It's eventually possible to shortcut multiple imports | ||
| using the following pattern. | ||
| However, it won't be fully compatible | ||
| with current standard `import` proposal as it is today. | ||
| ```js | ||
| Promise.all([ | ||
| 'fs', | ||
| 'path' | ||
| ].map( | ||
| m => module.import(m) | ||
| )).then(modules => {}); | ||
| ``` | ||
|
|
||
| ## Unleashing asynchronous exports | ||
| While good old modules will keep being usable, | ||
| new modules might define themselves as asynchronous | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't require new API
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It requires a standard way to load them as such, hence the proposal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's no better then doing
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is. It hides the implementation details that could be asynchronous require behind the scene, enabled already on the web. Your one is still an explicit synchronous intent that has no future compatibility as moving forward pattern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only works if you use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's like saying: this only works if everyone will use I don't like argumentations based on speculations. This is a moving-forward pattern, someone might do it, specially on the web. One of the clear goal is improving universal modules. Synchronous I won't comment further about speculations on what you think everyone does or want. I hope this can be kept as regular conversation and exchange of ideas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not true, this will never work with
This is a very bad position when proposing some API changes. You could propose something useless and ugly-looking and argue that someone might want to use that and saying anything else is speculation.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've explained already why this is needed so you already have someone that wants to do things this way and did it already. Patterns are usually abused by developers so if there's a common one that works with everything, that pattern will be for the sake of consistency.
dynamic Transpilers could target this method to fallback, developers can easily migrate to asynchronous modules simply using this method, which works already in NodeJS 0.12 It's a migration pattern, and it doesn't exist yet, so it's pointless to say "nobody and no one" since few already liked the proposal (through the blogpost, through the repository). As summary, mine is not a position at all, is a way to have a conversation based on facts or possible issues, and speculating about the future is, in my opinion, a non-argument. |
||
| by exporting a `Promise` rather than directly the module. | ||
| ```js | ||
| // example: dir.js | ||
| module.exports = Promise.all([ | ||
| module.import('fs'), | ||
| module.import('path') | ||
| ]).then(([fs, path]) => { | ||
| // once all dependencies have been imported | ||
| // return the module that should be imported | ||
| return { | ||
| list(dir) { | ||
| return new Promise((res, rej) => { | ||
| fs.readdir( | ||
| path.resolve(dir), | ||
| (err, list) => { | ||
| if (err) rej(err); | ||
| else res(list); | ||
| } | ||
| ); | ||
| }); | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| // import example | ||
| module.import('./dir').then(dir => { | ||
| dir.list('.').then(console.log); | ||
| }); | ||
| ``` | ||
|
|
||
| ## Asynchronous exports: why? | ||
| The example with core modules is mostly for | ||
| copy and paste testability sake only, | ||
| the truth is that many modules have | ||
| asynchronous initialization behind databases connections, | ||
| remote API calls, and other non blocking operations. | ||
|
|
||
| New modules created with async `import(...)` in mind | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, they can't , until top level await is in the spec
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This proposal works already with async export and is future friendly with top level async. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What you propose has no standard equivalent, i.e. someone who relies on
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't mater, this is not, and cannot be, a polyfill for The day If it's not, it'll be rather about how you export, than how you import, since the protocol Promise based has been confirmed at Stage 3 and won't change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Top level await is bad |
||
| can early reject when needed, instead of emitting | ||
| or throwing unhandled errors in the wild at distance. | ||
|
|
||
| Importing a list of 20 modules at once, would be | ||
| easy to _catch_ all at once and react accordingly, | ||
| as opposite of handling every single module possible | ||
| non blocking initialization a part. | ||
|
|
||
| On top of that, on the client side, | ||
| every module would most likely be preferred as asynchronous, | ||
| so that this proposal can improve Universal-JS-ability even more. | ||
|
|
||
| ## Why not as global function ? | ||
| The `import` word is reserved. | ||
| Unless the community wants to wait until the official standard | ||
| is finalized and the v8 change shipped, | ||
| using a `global.import(path)` does not seem even | ||
| aligned with the concept of module, which should never even use | ||
| `global` reference if not to quickly feature test something, | ||
| and does not seem to be universally friendly, | ||
| since `global` is not standard outside NodeJS world. | ||
|
|
||
| ## Why polluting `module` and not `require` ? | ||
| There are at least two reasons: | ||
|
|
||
| 1. the Web has a role too | ||
| 2. historical confusion and semantics | ||
|
|
||
| The current proposal is aligned with what | ||
| `<script type="module">` should also ship on browsers. | ||
| This means that the word `module` is already part | ||
| of both ECMAScript and Web specifications. | ||
|
|
||
| The word `require`, and the presence of an extra callback | ||
| that is strictly coupled with synchronous CommonJS only, | ||
| something not considered by dynamic `import` at all | ||
| and actually warned in console when similarly operated | ||
| on the main thread of any Web page, would only create confusion | ||
| in terms of what `module` and `import` means for the present | ||
| and the future of the language, in a platform independent way. | ||
|
|
||
| On top of that, `require` function has been historically | ||
| [abused](https://www.npmjs.com/package/require.async) or | ||
| [polluted](http://requirejs.org/docs/api.html#data-main) | ||
| in an era where `Promise`, as well as `async` and `await` | ||
| patterns, weren't even discussed in the JS community. | ||
|
|
||
| Accordingly, adding yet another method to a function | ||
| which name and semantics are unknown for the rest of | ||
| the JS ecosystem, does not look like a good choice | ||
| in terms of new, future friendly, module feature. | ||
|
|
||
| On the other hand, being `module` where developers | ||
| define their exports, I don't see any confusion | ||
| in being `module` where developers can import | ||
| their dependencies. | ||
|
|
||
| After all, they are going to "_import module_" | ||
| or "_export module_", and not going to | ||
| "_require an import of a module_" since they never | ||
| "_required an export of a module_". | ||
|
|
||
| ## Why as core feature ? Why not transpilers ? | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By the way, since you insist this should be core instead of being a userland package, I think this section should be amended to address that question.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section is about transpilers. I can add another one about modules.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I was under the impression that this section is more about "Why as core feature " than "Why not transpilers" |
||
| Transpilers are a great resource to have new | ||
| language features implemented in what's there already. | ||
|
|
||
| Unfortunately these can also have bugs, like it's been | ||
| for [classes](https://github.com/babel/babel/issues/4480) | ||
| since ever, so that transpilers become an issue, a stopper, | ||
| or a feature that cannot be fully trusted instead. | ||
|
|
||
| Using transpilers since ES2015 had various side effects | ||
| due un-polyfillable new features such `Symbol`, `WeakMap`, | ||
| `class`, `super()`, and shorthand methods without prototype. | ||
|
|
||
| Accordingly, requiring transpilers as mandatory tool | ||
| to import asynchronously a module, is not necessarily | ||
| the most secure, reliable, or plausible advice. | ||
|
|
||
| Since this proposal has been tested to be reliable | ||
| down to NodeJS 0.12, the first one that shipped with | ||
| native `Promise`, I don't see why we should lose | ||
| this opportunity to make every CommonJS user | ||
| capable right away to export, and import, | ||
| asynchronous modules that are future friendly. | ||
|
|
||
| ## What about proposing this as standard ? | ||
| I have already mentioned these points in one open issue [1]. | ||
|
|
||
| In the very same thread, there are other developers believing | ||
| `module`, which is luckily not a reserved word, is the best | ||
| object to host `import` capabilities and per each module, | ||
| like it is already in CommonJS and NodeJS. | ||
|
|
||
| ## How can this work on the Web ? | ||
| I have already created a tiny ~1KB utility that brings | ||
| everything discussed in here to browsers too (all of them) [2]. | ||
|
|
||
| Despite my utility usage, if the proposal will end up agreeing | ||
| on using a `module.import(path)` method, instead of `import(path)`, | ||
| there'll still be a `module` object to eventually attach an export, | ||
| meaning the future code won't break even for early adopters. | ||
|
|
||
| ## How can old NodeJS version import new modules ? | ||
| They can add the top level implementation in their main entry point, | ||
| as polyfill for all required modules, or they can simply | ||
| install new modules and require them as such: | ||
| ```js | ||
| var asyncModule = Promise.resolve( | ||
| require('new-module') | ||
| ).then(function (newModule) { | ||
| // do something | ||
| }); | ||
| ``` | ||
| Following the same approach, NodeJS 0.12 env can also already | ||
| create and export asynchronous module. | ||
|
|
||
| ## When `import` will be official, what happens ? | ||
| The same that happened until now with `require`, | ||
| the previously mentioned transpilers will have a core | ||
| utility to fallback the new syntax that cannot possibly be | ||
| natively polyfilled in any JS environment as it is now. | ||
|
|
||
|
|
||
|
|
||
| [1] https://github.com/tc39/proposal-dynamic-import/issues/35#issuecomment-274561912 | ||
|
|
||
| [2] https://github.com/WebReflection/common-js | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look closely, this proposal is about bringing dynamic loading, not asynchronous loading. It is motivated by examples, where module specifier is not statically known. That's something that is handled by CommonJS as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples in the same page shows asynchronous imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens to be asynchronous, but it's not what proposal is about. Static ES6 imports can be loaded asynchronously already, so it's not what's lacking