Skip to content

Instantly share code, notes, and snippets.

@Rich-Harris
Created June 10, 2016 18:15
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Rich-Harris/3107038d3d3f9ed956a1ac9fd6e75b97 to your computer and use it in GitHub Desktop.
Save Rich-Harris/3107038d3d3f9ed956a1ac9fd6e75b97 to your computer and use it in GitHub Desktop.

The case against nested import declarations

Context

This document by Ben Newman, and the ensuing Twitter conversation. You should read those first.

Who am I?

I, Rich Harris, am

  • just some guy
  • not involved in standards efforts at all
  • tbh I don't even work in the technology industry
  • but I did write a couple of module bundlers (Esperanto and its successor, Rollup, both of which have had modest success)
  • so I do have a dog in this fight
  • though Ben is a much smarter programmer/human than me, so he's probably right and I'm probably wrong

Now that's out of the way: Ben has lots of good examples of code that is made cleaner and clearer by allowing import declarations inside blocks. And this...

To put it in even stronger terms, if we do not allow ourselves to nest import declarations, then serious applications will never be able to stop using the CommonJS require function, and JavaScript modules will remain an awkward hybrid of modern and legacy styles.

Is that a future we can tolerate?

...definitely makes you sit up and take notice. But it's not actually true:

if ( weNeedFoo ) {
  loader.import( './foo.js' ).then( foo => {
    // tada! conditional dynamic loading.
    // note: i'm not at all sure that this is
    // what it'll look like. will loader be
    // called `loader`? will it take a single
    // string as its argument? i actually have
    // no idea. But you get the general point.
  });
}

That's definitely not as pleasant or convenient (or as statically analysable, FWIW) as an import declaration, but it's pretty clear what's going on here. No surprises about order of execution and whatnot.

Guy suggests that we could treat an inline import declaration as syntactic sugar:

// this...
if ( weNeedFoo ) {
  import foo from './foo.js';
}

// ...is equivalent to this:
if ( weNeedFoo ) {
  const foo = await loader.import( './foo.js' );
}

But that's not true, because you can only use await inside an async function. You could make the whole module async, but then you can't use import inside a non-async function:

function doFoo () {
  import foo from './foo.js';
  foo();
}

// doThis and doThat happen in the same tick, which means
// we can't sit around and wait for `foo`
doThis();
doFoo();
doThat();

In Node, that's sort of okay, because (in spite of Noders constantly chastising developers for using synchronous functions where async equivalents exist) require is synchronous. So we can just use that. But across a network? Nope.

So the implication is that we would make all the modules available (loaded, but not necessarily executed yet) at initial execution time. In other words, for that conditional import to work in a browser, we'd have to defensively load the entire speculative dependency graph – all the modules that we might need, plus all the modules they might need, and so on – just so foo could be there when we needed it.

This is how Browserify works, by the way – this code...

const lol = Math.random() < 0.5 ?
  require( './foo.js' ) :
  require( './bar.js' );

...will cause the contents of both foo.js and bar.js to be included in your bundle. Which probably isn't what you wanted.

Maybe we have nested imports, and just get into the habit of not using them (and using loader.import instead) in code that's intended for the browser. But one of the great things about ES2015 modules is that they will make sharing code between server and client much easier. The only way nested import declarations really make sense is if we have a node-centric view of the world.

It's entirely possible that I've just completely misunderstood the whole thing, in which case sorry. Like Ben I believe that native modules will have a more significant impact on developer happiness and productivity than any other recent or upcoming JavaScript feature, so it's important we get this stuff right.

@benjamn
Copy link

benjamn commented Jun 23, 2016

What happens if you have some code that imports a module at the top level

import foo from "./foo";

but then some other code that imports the same module asynchronously?

loader.import("./foo").then(fooExports => {
  let foo = fooExports.default;
});

Doesn't that force the module to be registered via the loader API anyway, and not inlined the way Rollup would currently handle the synchronous top-level import declaration?

If any subset of your modules might have to be registered in a way that allows for selective/lazy loading, then it seems Rollup is going to have to support that style one way or another, and then it's not a big leap to supporting nested import declarations.

@Rich-Harris
Copy link
Author

Short answer is I don't yet know what Rollup would do in that scenario – we'd cross that bridge when we came to it. To my mind this isn't about what Rollup (which is really just a bit of duct tape to hold stuff together until modules are natively supported) would do, it's about what browsers would do.

And I think we need to think about the browser first, Node second (something we've lost the habit of in the JS community) – the designers of ES modules did focus on browsers, and settled on asynchronous loading to solve the problem of conditional/dynamic imports.

Just so we're on the same page: am I right in thinking that treating a nested import as syntactic sugar for await import(...) is a non-starter, for the reasons outlined above? Or am I missing something?

@benjamn
Copy link

benjamn commented Jun 23, 2016

Here's what a browser should do when evaluating a module A:

  1. Take advantage of the fact that all ES2015 import declarations have statically analyzable source module identifiers (the string literal after from) to determine the set of modules that might be imported by A.
  2. Make sure those modules, and any other modules they might import, have been fetched from the server.
  3. Evaluate the A module, which is now free to import synchronously, because the browser guarantees all the modules it might import are immediately available.

The only asynchronous part of this story is the part where the browser has to fetch modules from the server, but that doesn't have to happen while A is executing, and might not even be necessary unless you're trying to do some sort of fancy code splitting.

@benjamn
Copy link

benjamn commented Jun 23, 2016

Just so we're on the same page: am I right in thinking that treating a nested import as syntactic sugar for await import(...) is a non-starter, for the reasons outlined above? Or am I missing something?

Right, it's a non-starter because await is only legal in async functions, though I happen to think that's another restriction worth reconsidering: http://benjamn.github.io/goto2015-talk

That's a pretty controversial opinion, though, and I definitely don't want to wait until JavaScript has coroutines (aka await anywhere) to start talking about nested import statements.

@benjamn
Copy link

benjamn commented Jun 23, 2016

I will admit loader.import(x, parentModuleIdentifier) is useful in those rare cases when the value of x is truly dynamic, but waiting until runtime to figure out what you're importing seems like a recipe for terrible page load times…

@Rich-Harris
Copy link
Author

Make sure those modules, and any other modules they might import, have been fetched from the server.

Really? A mobile phone on a bad 3G network shouldn't evaluate JavaScript until it's loaded nice-to-have-but-frankly-non-essential stuff like analytics code, or routes that you may or may not visit? I don't agree.

though I happen to think that's another restriction worth reconsidering: http://benjamn.github.io/goto2015-talk

you're a madman :-)

waiting until runtime to figure out what you're importing seems like a recipe for terrible page load times…

I'd argue that pre-emptively loading everything would be much worse for load times, in typical situations. Truly dynamic imports are rare, but that works in our favour – for loader.import calls with string literals, a server could push modules, or a service worker could get busy fetching modules it thinks it might need (depending on what the user ends up doing).

@benjamn
Copy link

benjamn commented Jun 23, 2016

So I think the most natural (ES2015) way to express that a module doesn't have to be fetched immediately is to nest the import declaration in an async function, as I explore here.

But of course you can also import it with loader.import, and interact with the resulting Promise however you like! That's totally still legal in a world of nested import declarations, and might (or might not) be a better idea on mobile. Nothing about nested import declarations prevents you from using loader.import where it makes sense to do so.

@Rich-Harris
Copy link
Author

So you're saying that this...

describe("fancy feature #5", () => {
  import { strictEqual } from "assert";

  it("should work on the client", () => {
    import { check } from "./client.js";
    strictEqual(check(), "client ok");
  });

  it("should work on the client", () => {
    import { check } from "./server.js";
    strictEqual(check(), "server ok");
  });

  it("should work on both client and server", () => {
    import { check } from "./both.js";
    strictEqual(check(), "both ok");
  });
});

...could desugar to this...

import { strictEqual } from "assert";
import { check as checkClient } from "./client.js";
import { check as checkServer } from "./server.js";
import { check as checkBoth } from "./both.js";

describe("fancy feature #5", () => {
  it("should work on the client", () => {
    strictEqual(checkClient(), "client ok");
  });

  it("should work on the client", () => {
    strictEqual(checkServer(), "server ok");
  });

  it("should work on both client and server", () => {
    strictEqual(checkBoth(), "both ok");
  });
});

...but this...

describe("fancy feature #5", async () => {
  import { strictEqual } from "assert";

  it("should work on the client", async () => {
    import { check } from "./client.js";
    strictEqual(check(), "client ok");
  });

  it("should work on the client", async () => {
    import { check } from "./server.js";
    strictEqual(check(), "server ok");
  });

  it("should work on both client and server", async () => {
    import { check } from "./both.js";
    strictEqual(check(), "both ok");
  });
});

...could desugar to this?

describe("fancy feature #5", async () => {
  const { strictEqual } = await loader.import( "assert" );

  it("should work on the client", async () => {
    const { check } = await loader.import( "./client.js" );
    strictEqual(check(), "client ok");
  });

  it("should work on the client", async () => {
    const { check } = await loader.import( "./server.js" );
    strictEqual(check(), "server ok");
  });

  it("should work on both client and server", async () => {
    const { check } = await loader.import( "./both.js" );
    strictEqual(check(), "both ok");
  });
});

And a linter or a bundler could be configured to warn or throw an error if you have a nested import inside a non-async function? Interesting. That's certainly a big improvement on having to serve the whole lot up front regardless, though it does present ample room for confusion.

But of course you can also import it with loader.import, and interact with the resulting Promise however you like!

Yes, of course. For me it's just about reducing conceptual uncertainty and limiting opportunities to do the wrong thing (i.e. forcibly bundling non-essential code). Most people who haven't been obsessing over ES modules for the last several months will find this sort of thing highly confusing. Personally I value obviousness and unambiguousness over nicer syntax, when the two are in conflict.

@benjamn
Copy link

benjamn commented Jun 23, 2016

Yeah, if we imagine await is legal at the top level of modules, then all of the following import declarations could desugar to await loader.import(...), though that doesn't have to be how they're actually implemented:

// This is obviously legal.
import { a } from "./a";
let sum = a;

if (weNeedB) {
  // An await expression should be legal here, if modules are like async functions.
  import { b } from "./b";
  sum += b;
}

async function add(weNeedD) {
  // Since await is legal here, this seems fine.
  import { c } from "./c";
  sum += c;

  if (weNeedD) {
    // Here too!
    import { d } from "./d";
    sum += d;
  }

  return sum;
}

add(true).then(result => console.log(result));

I think that just leaves import declarations in non-async functions, whose modules would need to be fetched synchronously before evaluating the parent module, just in case the non-async function gets called immediately.

I'm reluctant to take the extra step of forbidding import declarations in non-async functions, since supporting them shouldn't be all that hard. Discouraging them feels like a job for linters (as you suggested), rather than the language itself.

Now I'm wondering which proposal is more likely to fly with TC39:

  • import is legal where await is legal, but then we also have to make await legal at the top level of modules, or
  • import is legal everywhere, but it doesn't desugar to await in all cases.

I would be satisfied with either proposal.

@benjamn
Copy link

benjamn commented Jun 23, 2016

Just to clarify: I definitely do not think the ECMAScript spec should talk about the desugaring, since that's an implementation detail, but I totally understand the desire for the spec to allow for a simple desugaring story. Straightforward desugaring was one of the guiding principles behind ES2015 class syntax, if I recall, and I think it has helped the adoption of classes.

@Rich-Harris
Copy link
Author

Are you okay with a delay between a() and b() here?

async function foo ( maybe ) {
  if ( maybe ) {
    a();
    import b from './b.js';
    b();
  }

  // ...
}

I wouldn't want any ambiguity about whether import was equivalent to an await in a situation like that (and I think a lot of people would be surprised by a delay regardless). Only way to avoid that would be to hoist imports to the top of the nearest async function context, at which point it's no longer conditional.

Relatedly, is b() in some kind of temporal dead zone here?

async function foo ( maybe ) {
  if ( maybe ) {
    b();
    import b from './b.js';
  }

  // ...
}

@benjamn
Copy link

benjamn commented Jun 24, 2016

I've been persuaded by @sokra that hoisting is important for import declarations. That's what inspired this section. If import declarations are hoisted, then there's no delay between a() and b() in your first example, and in your second example b() would be called after b is imported, which is likely what you want.

Only way to avoid that would be to hoist imports to the top of the nearest async function context, at which point it's no longer conditional.

The way I've implemented hoisting in reify, import declarations are hoisted to the beginning of the enclosing block, not to the beginning of the function, so they are still conditional. Under that interpretation, it seems natural that the imported symbols should be scoped like let variables, so they aren't visible outside the enclosing block. I think that prevents any TDZ issues.

@Rich-Harris
Copy link
Author

Hoisting to the enclosing block doesn't remove the ambiguity about whether or not there's a delay between a() and b(), it just moves it to a different place:

async function foo ( maybe ) {
  a();
  if ( maybe ) {
    import b from './b.js';
    b();
  }

  // ...
}

@trusktr
Copy link

trusktr commented Jun 24, 2016

How does the loader load modules simultaneously when possible? Maybe imports within the same block,

if (something) {
  import { a } from "./a";
  import { b } from "./b";
  import { c } from "./c";
}

desugars to something like

if (something) {
  const __modules__ = await loader.import('./a', './b', './c')
  const {a} = __modules__['./a']
  const {b} = __modules__['./b']
  const {c} = __modules__['./c']
}

so that the loader can determine the most efficient way to load the modules as simultaneously as possible?

@benjamn
Copy link

benjamn commented Jun 27, 2016

@trusktr I think the idiomatic desugaring would be

if (something) {
  const [a, b, c] = await Promise.all(
    loader.import("./a"),
    loader.import("./b"),
    loader.import("./c")
  );
}

I definitely agree with you that sequential asynchronous operations seem inefficient, but I'm not entirely sure it's safe to parallelize module evaluation. What you really want to parallelize is the fetching of the module sources, and then the imports themselves can be sequential and synchronous.

@Rich-Harris
Copy link
Author

@benjamn @trusktr You're right, it's definitely not safe to parallelize evaluation, and by extension I think the Promise.all form is a non-starter.

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