Merge pull request #1 from meteor/fix/check-scope-when-wrapping

Check scope before wrapping assignment benjamn#326

.gitignore

@@ -2,4 +2,6 @@
 *.log
 test/.cache
 test/**/*.gz
-node_modules
+/node_modules
+.reify-cache
+.idea

README.md

@@ -1,4 +1,4 @@
-# re&middot;i&middot;fy <sub>_verb, transitive_</sub> &nbsp; [![Build Status](https://github.com/benjamn/reify/actions/workflows/node.js.yml/badge.svg)](https://github.com/benjamn/reify/actions/workflows/node.js.yml)
+# re&middot;i&middot;fy <sub>_verb, transitive_</sub> &nbsp; [![Build Status](https://github.com/meteor/reify/actions/workflows/node.js.yml/badge.svg)](https://github.com/meteor/reify/actions/workflows/node.js.yml)
 
 **re&middot;i&middot;fied** <sub>past</sub> &nbsp; **re&middot;i&middot;fies** <sub>present</sub> &nbsp; **re&middot;i&middot;fy&middot;ing** <sub>participle</sub> &nbsp; **re&middot;i&middot;fi&middot;ca&middot;tion** <sub>noun</sub> &nbsp; **re&middot;i&middot;fi&middot;er** <sub>noun</sub>
 
@@ -10,17 +10,17 @@
 Usage
 ---
 
-  1. Run `npm install --save reify` in your package or app directory. The
+  1. Run `npm install --save @meteorjs/reify` in your package or app directory. The
      `--save` is important because reification only applies to modules in
-     packages that explicitly depend on the `reify` package.
-  2. Call `require("reify")` before importing modules that contain `import`
+     packages that explicitly depend on the `@meteorjs/reify` package.
+  2. Call `require("@meteorjs/reify")` before importing modules that contain `import`
      and `export` declarations.
 
-You can also easily `reify` the Node REPL:
+You can also easily `@meteorjs/reify` the Node REPL:
 
 ```sh
 % node
-> require("reify")
+> require("@meteorjs/reify")
 {}
 > import { strictEqual } from "assert"
 > strictEqual(2 + 2, 5)

examples/babel-parser-example/index.js

@@ -1,2 +1,2 @@
-require("reify/node");
+require("@meteorjs/reify/node");
 module.exports = require("./module.js");

examples/babel-parser-example/package.json

@@ -24,7 +24,7 @@
     "node": ">=4"
   },
   "dependencies": {
-    "reify": "^0.18.1"
+    "@meteorjs/reify": "0.23.0-beta.0"
   },
   "reify": {
     "parser": "reify/lib/parsers/babel.js"

examples/custom-cache-example/index.js

@@ -1,2 +1,2 @@
-require("reify/node");
+require("@meteorjs/reify/node");
 module.exports = require("./module.js");

examples/custom-cache-example/package.json

@@ -24,7 +24,7 @@
     "node": ">=4"
   },
   "dependencies": {
-    "reify": "^0.5.1"
+    "@meteorjs/reify": "0.23.0-beta.0"
   },
   "reify": {
     "cache-directory": ".reify-custom-cache"

examples/top-level-example/index.js

@@ -1,2 +1,2 @@
-require("reify/node");
+require("@meteorjs/reify/node");
 module.exports = require("./module.js");

examples/top-level-example/package.json

@@ -24,7 +24,7 @@
     "node": ">=4"
   },
   "dependencies": {
-    "reify": "^0.5.1"
+    "@meteorjs/reify": "0.23.0-beta.0"
   },
   "reify": {
     "parser": "reify/lib/parsers/top-level.js"

lib/assignment-visitor.js

@@ -80,26 +80,59 @@ function assignmentHelper(visitor, path, childName) {
   const assignedNames = utils.getNamesFromPattern(child);
   const nameCount = assignedNames.length;
 
-  // Wrap assignments to exported identifiers with `module.runSetters`.
+  let scope = null;
+  function inModuleScope(name) {
+    if (scope === null) {
+      scope = path.getScope();
+    }
+
+    return !scope || scope.find_owner(name).parent === null;
+  }
+
+  let modifiedExports = [];
+
+  // Identify which exports if any are modified by the assignment.
   for (let i = 0; i < nameCount; ++i) {
-    if (visitor.exportedLocalNames[assignedNames[i]] === true) {
-      wrap(visitor, path);
-      break;
+    let name = assignedNames[i];
+    if (
+      visitor.exportedLocalNames[name] &&
+      inModuleScope(name)
+    ) {
+      modifiedExports.push(...visitor.exportedLocalNames[name]);
     }
   }
+
+  // Wrap assignments to exported identifiers with `module.runSetters`.
+  if (modifiedExports.length > 0) {
+    wrap(visitor, path, modifiedExports);
+  }
 }
 
-function wrap(visitor, path) {
+function wrap(visitor, path, names) {
   const value = path.getValue();
 
   if (visitor.magicString !== null) {
+    let end = ')';
+    if (names) {
+      end = `,[${names.map(n => `"${n}"`).join(',')}])`;
+    }
+
     visitor.magicString.prependRight(
       value.start,
       visitor.moduleAlias + ".runSetters("
-    ).appendLeft(value.end, ")");
+    ).appendLeft(value.end, end);
   }
 
   if (visitor.modifyAST) {
+    let args = [value];
+    if (names) {
+      let array = {
+        type: "ArrayExpression",
+        elements: names.map(n => ({ type: "StringLiteral", value: n }))
+      };
+      args.push(array);
+    }
+
     path.replace({
       type: "CallExpression",
       callee: {
@@ -113,7 +146,7 @@ function wrap(visitor, path) {
           name: "runSetters"
         }
       },
-      arguments: [value]
+      arguments: args
     });
   }
 }

lib/fast-path.js

@@ -5,6 +5,7 @@
 
 const assert = require("assert");
 const utils = require("./utils.js");
+const periscopic = require('periscopic');
 const brandKey = "reify-fast-path";
 const brand = typeof Symbol === "function"
   ? Symbol.for(brandKey)
@@ -15,6 +16,7 @@ class FastPath {
     assert.notStrictEqual(ast, null);
     assert.strictEqual(typeof ast, "object");
     this.stack = [ast];
+    this._scope = null;
   }
 
   static isInstance(value) {
@@ -102,6 +104,21 @@ class FastPath {
     return getStackAt(this, 1);
   }
 
+  getScope() {
+    if (!this._scope) {
+      this._scope = periscopic.analyze(this.stack[0]);
+    }
+
+    let node;
+    let pos = 0;
+    while (node = getNodeAt(this, pos++)) {
+      let scope = this._scope.map.get(node);
+      if (scope) {
+        return scope;
+      }
+    }
+  }
+
   replace(newValue) {
     const s = this.stack;
     const len = s.length;

lib/import-export-visitor.js

@@ -457,12 +457,10 @@ function addExportedLocalNames(visitor, specifierMap) {
     const localCount = locals.length;
 
     for (let j = 0; j < localCount; ++j) {
-      // It's tempting to record the exported name as the value here,
-      // instead of true, but there can be more than one exported name
-      // per local variable, and we don't actually use the exported
-      // name(s) in the assignmentVisitor, so it's not worth the added
-      // complexity of tracking unused information.
-      exportedLocalNames[locals[j]] = true;
+      if (exportedLocalNames[locals[j]] === undefined) {
+        exportedLocalNames[locals[j]] = [];
+      }
+      exportedLocalNames[locals[j]].push(exported);
     }
   }
 }

lib/runtime/index.js

@@ -117,8 +117,8 @@ function moduleExportAs(name) {
 // the module system is about to return module.exports from require. This
 // might happen more than once per module, in case of dependency cycles,
 // so we want Module.prototype.runSetters to run each time.
-function runSetters(valueToPassThrough) {
-  Entry.getOrCreate(this.id, this).runSetters();
+function runSetters(valueToPassThrough, names) {
+  Entry.getOrCreate(this.id, this).runSetters(names, true);
 
   // Assignments to exported local variables get wrapped with calls to
   // module.runSetters, so module.runSetters returns the

node/utils.js

@@ -11,6 +11,8 @@ const FastObject = require("../lib/fast-object.js");
 const PkgInfo = require("./pkg-info.js");
 const SemVer = require("semver");
 
+const REIFY_PACKAGE_NAME = '@meteorjs/reify';
+
 const DEFAULT_PKG_CONFIG = {
   "cache-directory": ".reify-cache",
   parser: void 0,
@@ -23,8 +25,8 @@ const reifySemVer = require("./version.js");
 
 function getReifyRange(json, name) {
   const entry = json[name];
-  return utils.isObject(entry) && hasOwn.call(entry, "reify")
-    ? SemVer.validRange(entry.reify)
+  return utils.isObject(entry) && hasOwn.call(entry, REIFY_PACKAGE_NAME)
+    ? SemVer.validRange(entry[REIFY_PACKAGE_NAME])
     : null;
 }
 
@@ -105,7 +107,11 @@ function maxSatisfying(versions, range) {
   if (cacheKey in maxSatisfyingCache) {
     return maxSatisfyingCache[cacheKey];
   }
-  return maxSatisfyingCache[cacheKey] = SemVer.maxSatisfying(versions, range);
+  return maxSatisfyingCache[cacheKey] = SemVer.maxSatisfying(
+    versions,
+    range,
+    { includePrerelease: range === '*' }
+  );
 }
 
 exports.maxSatisfying = maxSatisfying;
@@ -118,7 +124,7 @@ function readPkgInfo(dirPath) {
     return null;
   }
 
-  const reify = pkgJSON.reify;
+  const reify = pkgJSON[REIFY_PACKAGE_NAME];
 
   if (reify === false) {
     // An explicit "reify": false property in package.json disables