Skip to content
This repository was archived by the owner on Jul 31, 2018. It is now read-only.
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
241 changes: 241 additions & 0 deletions XXX-module-import.md
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.

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.

Copy link
Author

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

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


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'),
Copy link
Member

@joyeecheung joyeecheung Feb 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proposal should be clear about whether module.import() is blocking or not, if not, people will be aware that they are not supposed to mutate shared external state in the module when it is being imported, and they will probably need to change these behavior in their code. If it is blocking(behavior of the old require), then people can be sure they don't have to change their code, and they would know the order of these modules code being executed is deterministic. And this behavior should be in line with the future import() implementation in core.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea is to have a non blocking module.import() now ensured by the updated code that uses Promise.resolve().then upfront so the import is always asynchronous.

What the implementation does behind the scene shouldn't be a developer concern: they must be aware the import returns asynchronously the content, no matter which content it is

Copy link
Member

@joyeecheung joyeecheung Feb 6, 2017

Choose a reason for hiding this comment

The 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 c is a ESM being import()ed

Copy link
Author

Choose a reason for hiding this comment

The 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 ?

Copy link
Member

@joyeecheung joyeecheung Feb 6, 2017

Choose a reason for hiding this comment

The 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 c in nondeterministic.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Author

@WebReflection WebReflection Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESM must export at the top level.

once dynamic import will be in place, considering that such feature is proposed with the Web in mind, and considering that the Web will inevitably have asynchronous dependencies, are you sure ESM won't be capable of doing the following?

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 Promise.resolve() called sequently, there should never be a race condition case if modules are exported synchronously.

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)
]);

Copy link
Member

@joyeecheung joyeecheung Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, being the current logic based on synchronous require and the Promise.resolve() called sequently, there should never be a race condition case if modules are exported synchronously.

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 synchronousrequire intentional? Will it ever decide to switch to something else, like another loader, which could, for example, usefs.readfile to separate the reading and the parsing/execution? If it is intentional, this should be pointed out so people know the cost of transition to this pattern, if not, this should be pointed out too, so people would know this is just a coincidence and there is no guarantee about it.

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.

Copy link
Author

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't require new API

Copy link
Author

Choose a reason for hiding this comment

The 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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's no better then doing require('foo').then

Copy link
Author

Choose a reason for hiding this comment

The 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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works if you use module.import instead of require all the time, but no one will do that. There is no "synchronous intent". Module exports a promise and I need to call then. It will work exactly the same with ES modules, so I don't understand what "future compatibility" and "moving forward" are you talking about

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but no one will do that.

It's like saying: this only works if everyone will use import() instead of require() but no one will do that.

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 require is why we need bundlers, AMD was born before Promise were there and now we have a standard proposal based on Promise.

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.
Thank you

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only works if everyone will use import() instead of require() but no one will do that.

Not true, this will never work with import() at all, so it's irrelevant.

import() is supposed to be used either when you don't know module specifier. If you don't you just use synchronous-looking import foo from 'foo'. So yes, you are not supposed to use import() for everything, and no one will use module.import for everything.

I won't comment further about speculations on what you think everyone does or want

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.

Copy link
Author

@WebReflection WebReflection Feb 5, 2017

Choose a reason for hiding this comment

The 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.

require is an old pattern that worked well but will be inevitably replaced by static import
not because I say so, because that's ES standard

dynamic import() exist because somebody wants to load dynamically modules and it enables asynchronous exports too, explained in this proposal why are needed, efficient, better, etc.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, they can't , until top level await is in the spec

Copy link
Author

Choose a reason for hiding this comment

The 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.

Choose a reason for hiding this comment

The 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 module.import won't be able to switch to import() easily

Copy link
Author

Choose a reason for hiding this comment

The 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 import
This is a moving-forward, backward compatible, future friendly, migration pattern.

The day import will works in all negines you target, if it's compatible 1:1 with this proposal, you'll just drop module. prefix.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is about transpilers. I can add another one about modules.

Copy link
Member

Choose a reason for hiding this comment

The 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