babel-plugin-mockable-imports - npm Package Compare versions

Comparing version 1.1.0 to 1.2.0

CHANGELOG.md

@@ -8,2 +8,9 @@ # Changelog
 
+## [1.2.0] - 2019-04-11
+
+- Make CommonJS imports which use destructuring mockable when the
+  babel-transform-object-destructuring plugin is enabled (#4)
+- Fix use of CommonJS imports if variable declarations before the last
+  CommonJS import in the file references them (#4)
+
 ## [1.1.0] - 2019-04-08
@@ -10,0 +17,0 @@

index.js

@@ -37,8 +37,22 @@ 'use strict';
   /**
+   * Create an `$imports.$add(alias, source, symbol, value)` method call.
+   */
+  function createAddImportCall(alias, source, symbol, value) {
+    return t.callExpression(
+      t.memberExpression(t.identifier('$imports'), t.identifier('$add')),
+      [
+        t.stringLiteral(alias),
+        t.stringLiteral(source),
+        t.stringLiteral(symbol),
+        value,
+      ],
+    );
+  }
+
+  /**
    * Return true if imports from the module `source` should not be made
    * mockable.
    */
-  function excludeImportsFromModule(source, state) {
-    const excludeList = state.opts.excludeImportsFromModules || EXCLUDE_LIST;
-    return excludeList.includes(source);
+  function excludeImportsFrom(source, excludeList = EXCLUDE_LIST) {
+    return source === helperImportPath || excludeList.includes(source);
   }
@@ -83,2 +97,56 @@
 
+  // Visitor which collects information about CommonJS imports in a variable
+  // declaration and populates `state.imports`.
+  const collectCommonJSImports = {
+    VariableDeclarator(path, {excludeImportsFromModules, imports}) {
+      // If the `require` is wrapped in some way, we just ignore it, since
+      // we cannot determine which symbols are being required without knowing
+      // what the wrapping expression does.
+      //
+      // An exception is made where the `require` statement is wrapped in
+      // a sequence (`var foo = (..., require('blah'))`) as some code coverage
+      // transforms do. We know in this case that the result will be the
+      // result of the require.
+      const init = lastExprInSequence(path.node.init);
+      if (!t.isCallExpression(init)) {
+        return;
+      }
+      const source = commomJSRequireSource(init);
+      if (!source) {
+        return;
+      }
+
+      if (excludeImportsFrom(source, excludeImportsFromModules)) {
+        return;
+      }
+
+      const id = path.node.id;
+      if (id.type === 'Identifier') {
+        const symbol = '<CJS>';
+        imports.push({
+          alias: id.name,
+          symbol,
+          source,
+          value: id,
+        });
+      } else if (id.type === 'ObjectPattern') {
+        // `var { aSymbol: localName } = require("a-module")`
+        for (let property of id.properties) {
+          if (!t.isIdentifier(property.value)) {
+            // Ignore destructuring more complex than a rename.
+            continue;
+          }
+          const symbol = property.key.name;
+          const value = property.value;
+          imports.push({
+            alias: property.value.name,
+            source,
+            symbol,
+            value,
+          });
+        }
+      }
+    },
+  };
+
   return {
@@ -125,21 +193,6 @@ visitor: {
 
-          // Generate `export const $imports = new ImportMap(...)`
-          const importMetaObjLiteral = t.objectExpression(
-            [...state.importMeta.entries()].map(([localIdent, meta]) =>
-              t.objectProperty(
-                t.identifier(localIdent.name),
-                t.arrayExpression([
-                  t.stringLiteral(meta.source),
-                  t.stringLiteral(meta.symbol),
-                  meta.value,
-                ]),
-              ),
-            ),
-          );
           const $importsDecl = t.variableDeclaration('const', [
             t.variableDeclarator(
               t.identifier('$imports'),
-              t.newExpression(t.identifier('ImportMap'), [
-                importMetaObjLiteral,
-              ]),
+              t.newExpression(t.identifier('ImportMap'), []),
             ),
@@ -155,4 +208,6 @@ ]);
 
+          const body = path.get('body');
+
           // Insert `$imports` declaration below last import.
-          const insertedNodes = state.lastImport.insertAfter(helperImport);
+          const insertedNodes = body[0].insertAfter(helperImport);
           insertedNodes[0].insertAfter($importsDecl);
@@ -164,3 +219,2 @@
           // must come after any `module.exports = <value>` assignments.
-          const body = path.get('body');
           body[body.length - 1].insertAfter(exportImportsDecl);
@@ -197,57 +251,29 @@
 
-      CallExpression(path, state) {
-        const node = path.node;
-        const source = commomJSRequireSource(node);
-        if (!source) {
+      VariableDeclaration(path, state) {
+        if (state.aborted) {
           return;
         }
 
-        if (excludeImportsFromModule(source, state)) {
+        // Ignore non-top level CommonJS imports.
+        if (path.parent.type !== 'Program') {
           return;
         }
 
-        // Ignore requires that are not part of the initializer of a
-        // `var init = require("module")` statement.
-        const varDecl = path.findParent(p => p.isVariableDeclarator());
-        if (!varDecl) {
-          return;
-        }
+        // Find CommonJS (`require(...)`) imports in variable declarations.
+        const imports = [];
+        const {excludeImportsFromModules} = state.opts;
+        path.traverse(collectCommonJSImports, {
+          excludeImportsFromModules,
+          imports,
+        });
 
-        // If the `require` is wrapped in some way, we just ignore it, since
-        // we cannot determine which symbols are being required without knowing
-        // what the wrapping expression does.
-        //
-        // An exception is made where the `require` statement is wrapped in
-        // a sequence (`var foo = (..., require('blah'))`) as some code coverage
-        // transforms do. We know in this case that the result will be the
-        // result of the require.
-        if (lastExprInSequence(varDecl.node.init) !== node) {
-          return;
-        }
-
-        // Ignore non-top level require expressions.
-        if (varDecl.parentPath.parent.type !== 'Program') {
-          return;
-        }
-
-        state.lastImport = varDecl.findParent(p => p.isVariableDeclaration());
-
-        // `var aModule = require("a-module")`
-        const id = varDecl.node.id;
-        if (id.type === 'Identifier') {
-          state.importMeta.set(id, {
-            symbol: '<CJS>',
+        // Register all found imports.
+        imports.forEach(({alias, source, symbol, value}) => {
+          state.importMeta.set(value, {
+            symbol,
             source,
-            value: id,
+            value,
           });
-        } else if (id.type === 'ObjectPattern') {
-          // `var { aSymbol: localName } = require("a-module")`
-          for (let property of id.properties) {
-            state.importMeta.set(property.value, {
-              symbol: property.key.name,
-              source,
-              value: property.value,
-            });
-          }
-        }
+          path.insertAfter(createAddImportCall(alias, source, symbol, value));
+        });
       },
@@ -288,3 +314,3 @@
           const source = path.node.source.value;
-          if (excludeImportsFromModule(source, state)) {
+          if (excludeImportsFrom(source, state.excludeImportsFromModules)) {
             return;
@@ -298,2 +324,6 @@ }
           });
+
+          path.insertAfter(
+            createAddImportCall(spec.local.name, source, imported, spec.local),
+          );
         });
@@ -313,8 +343,11 @@ },
 
-        // Ignore the reference in the generated `$imports` variable declaration.
-        const varDeclParent = child.findParent(p => p.isVariableDeclarator());
+        // Ignore the reference in generated `$imports.$add` calls.
+        const callExprParent = child.findParent(p => p.isCallExpression());
+        const callee = callExprParent && callExprParent.node.callee;
         if (
-          varDeclParent &&
-          varDeclParent.node.id.type === 'Identifier' &&
-          varDeclParent.node.id.name === '$imports'
+          callee &&
+          t.isMemberExpression(callee) &&
+          t.isIdentifier(callee.object) &&
+          callee.object.name === '$imports' &&
+          callee.property.name === '$add'
         ) {
@@ -355,2 +388,1 @@ return;
 };
-('use strict');

lib/helpers.js

@@ -33,2 +33,6 @@ "use strict";
   function ImportMap(imports) {
+    if (imports === void 0) {
+      imports = {};
+    }
+
     /**
@@ -42,2 +46,20 @@ * A mapping of import local name (or alias) to metadata about where
   /**
+   * Register an import.
+   *
+   * The `value` of the import will become available as a property named
+   * `alias` on this instance.
+   */
+
+
+  var _proto = ImportMap.prototype;
+
+  _proto.$add = function $add(alias, source, symbol, value) {
+    if (isSpecialMethod(alias)) {
+      return;
+    }
+
+    this.$meta[alias] = [source, symbol, value];
+    this[alias] = value;
+  }
+  /**
    * Replace true imports with mocks.
@@ -52,6 +74,4 @@ *
    */
+  ;
 
-
-  var _proto = ImportMap.prototype;
-
   _proto.$mock = function $mock(imports) {
@@ -120,5 +140,4 @@ var _this = this;
 
-    var proto = Object.getPrototypeOf(this);
     Object.keys(this.$meta).forEach(function (alias) {
-      if (proto.hasOwnProperty(alias)) {
+      if (isSpecialMethod(alias)) {
         // Skip imports which conflict with special methods.
@@ -137,2 +156,6 @@ return;
 
+function isSpecialMethod(name) {
+  return ImportMap.prototype.hasOwnProperty(name);
+}
+
 module.exports = {
@@ -139,0 +162,0 @@ ImportMap: ImportMap,

package.json

 {
   "name": "babel-plugin-mockable-imports",
-  "version": "1.1.0",
+  "version": "1.2.0",
   "description": "Babel plugin for mocking ES imports",
@@ -33,2 +33,3 @@ "main": "index.js",
     "@babel/plugin-syntax-jsx": "^7.2.0",
+    "@babel/plugin-transform-destructuring": "^7.4.3",
     "@babel/preset-env": "^7.4.3",
@@ -35,0 +36,0 @@ "chai": "^4.2.0",

README.md

@@ -51,3 +51,7 @@ # babel-plugin-mockable-imports
 
+By default the plugin will try to avoid processing test modules. See the
+section on limiting mocking to [specific
+files](#limiting-mocking-to-specific-files) for details.
 
+
 ### Basic usage in tests
@@ -100,2 +104,6 @@
 
+If the module you want to test uses CommonJS / Node style imports instead
+(`var someModule = require("some-module")`, see the [section on
+CommonJS](#commonjs-support).
+
 ### Mocking default exports
@@ -144,4 +152,4 @@
 As a convenience, the plugin by default skips any files in directories named
-"test", "__tests__" or subdirectories of directories with those names. This
-can be configured using the `excludeDirs` option.
+`test` or `__tests__` or their subdirectories. This can be configured using the
+`excludeDirs` option.
 
@@ -205,2 +213,19 @@ ### Options
 
+## Known issues and limitations
+
+- The plugin adds an export named `$imports` to every module it processes.
+  This may cause conflicts if you try to combine exports from multiple modules
+  using `export * from <module>`. [See
+  issue](https://github.com/robertknight/babel-plugin-mockable-imports/issues/2).
+  It can also cause problems if you have code which tries to loop over the
+  exports of a module and does not gracefully handle unexpected exports.
+- There is currently no support for dynamic imports, either using `import()`
+  to obtain a promise for a module, or calling `require` anywhere other than
+  at the top level of a module.
+
+## Troubleshooting
+
+If you encounter any problems using this plugin, please [file an
+issue](https://github.com/robertknight/babel-plugin-mockable-imports/issues).
+
 ## FAQ
@@ -213,5 +238,5 @@
 that arose when using [proxyquire](https://github.com/thlorenz/proxyquire). In
-particular:
+particular proxyquire:
 
- - It evaluates the module under test and all its dependencies with an empty
+ - Evaluates the module under test and all its dependencies with an empty
    module cache each time it is invoked. In some respects this is a useful
@@ -222,3 +247,3 @@ feature, but there is non-trivial overhead to doing this and
    different copies of the module come into contact with one another.
- - It is tied to Node and Browserify
+ - Is tied to Node and Browserify
 
@@ -225,0 +250,0 @@ There is another Babel plugin,

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

29234

increased by9.84%

Lines of code

470

increased by11.37%

Number of lines in readme file

261

increased by10.59%

Worsened metrics

Dev dependency count

9

increased by12.5%

No dependency changes