Meteor: Scope of imported variables (nested imports)

[klaussner]

I was trying to use nested imports and noticed that the scoping doesn't work as described in reify's README.md. In particular, a variable imported inside an if block has the scope of a var declaration, not a let declaration.

For example:

if (true) {
  import value from './file.js';
}

console.log(value);

logs the value of the imported file (instead of undefined) because the code is transpiled to:

if (true) {
  var value;

  module.import('./file.js', {
    "default": function(v) {
      value = v
    }
  });
}

console.log(value);

This leads to problems when using the same name for imports more than once, in different block scopes:

if (true) {
  import value from '/imports/a.js';
  console.log(value);
} else {
  import value from '/imports/b.js';
  console.log(value);
}

=> Errors prevented startup:

   While processing files with ecmascript (for target web.browser):
   client/client.js:5:9: /client/client.js: Duplicate declaration "value"

[benjamn]

The real reason for this is that Reify sits at the very end of the compiler toolchain, just before Node runs its version of eval if you're using the require("reify") strategy, or after all other Babel transforms if you're using it as part of meteor-babel.

Since Reify doesn't have the ability to transpile let declarations for older versions of Node, it seems like a bad idea for it to be generating new let declarations, if there's any chance that the JS engine does not support let natively. Unfortunately, Node 4 still doesn't support let outside of strict mode, so it's not enough to know that you're using Node 4. You might think that using import implies you're in a module, which implies strict mode, but in Meteor we let people opt into "use strict" for their modules, in case they're still using auto-imported "global" variables.

With all of that said, I think we may be able to fix this in https://github.com/meteor/babel by moving the Reify compilation step earlier in the Babel plugin pipeline, even if we can't fix it once and for all in Reify itself. Some Babel transforms insert additional import declarations, so we would have to reenable the Babel modules transform, but those import declarations won't need (or benefit from) Reify because they'll all be at the top level, and Babel will turn them into require calls. If this works, then Babel can transpile the let declarations created by Reify, and Reify will only have to handle the import declarations from your source code (not the ones Babel itself inserts).

[benjamn]

A fix for this issue will probably also fix https://github.com/meteor/meteor/issues/7662.

[benjamn]

Though tests were passing on #7975, a complete solution for this issue is going to have to wait for 1.4.3, since moving Reify before Babel means Reify must now handle syntax like

export const { foo, bar } = baz();

In other words, Reify really needs to sit somewhere in between transforms like the one that handles destructuring variable declarations and the one that handles let and const declarations. And given that those two transforms are closely related, the precise position of Reify in the transform pipeline is probably more important than I realized, which is mostly to say: Reify needs to become a proper Babel plugin.

The good news is that this issue (and #7662) will definitely be fixed when that happens.