fix identification of link files to take into account not only the import statements but also export (#98)

Author: davidfirst

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [unreleased]
 
+## [2.0.6-dev.1] - 2019-05-10
+
+- fix identification of link files to take into account not only the `import` statements but also `export`
+
 ## [2.0.5] - 2019-05-01
 
 - fix es6 with dynamic import to not show as missing dependencies

fixtures/build-tree/not-link-file/file-a.js

@@ -0,0 +1 @@
+import { varX } from './file-b';

fixtures/build-tree/not-link-file/file-b.js

@@ -0,0 +1,3 @@
+import { varX } from './file-c';
+
+// export { varX }; // uncomment to make the following test fail "fileA imports varX from fileB, fileB imports varX from fileC but not export it"

fixtures/build-tree/not-link-file/file-c.js

@@ -0,0 +1,2 @@
+const varX = 4;
+export { varX };

fixtures/build-tree/tree-shaking-cycle/is-string.js

@@ -1,2 +1,2 @@
 import { isString } from '.'; // cycle with ./index.js
-export default function() {}
+export default isString;

fixtures/path-map.json

@@ -27,7 +27,8 @@
         "importSpecifiers": [
           {
             "isDefault": true,
-            "name": "isString"
+            "name": "isString",
+            "exported": true
           }
         ]
       },
@@ -37,7 +38,8 @@
         "importSpecifiers": [
           {
             "isDefault": true,
-            "name": "isArray"
+            "name": "isArray",
+            "exported": true
           }
         ]
       }
@@ -52,7 +54,8 @@
         "importSpecifiers": [
           {
             "isDefault": true,
-            "name": "isString"
+            "name": "isString",
+            "exported": true
           }
         ]
       }
@@ -71,7 +74,8 @@
         "importSpecifiers": [
           {
             "isDefault": true,
-            "name": "isArray"
+            "name": "isArray",
+            "exported": true
           }
         ]
       }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "bit-javascript",
-  "version": "2.0.5",
+  "version": "2.0.6-dev.1",
   "scripts": {
     "flow": "flow; test $? -eq 0 -o $? -eq 2",
     "lint": "eslint src && flow check || true",

src/dependency-builder/build-tree.spec.js

@@ -129,5 +129,20 @@ describe('buildTree', () => {
         });
       });
     });
+    describe('fileA imports varX from fileB, fileB imports varX from fileC but not export it', () => {
+      let results;
+      before(async () => {
+        dependencyTreeParams.filePaths = [`${buildTreeFixtures}/not-link-file/file-a.js`];
+        results = await buildTree.getDependencyTree(dependencyTreeParams);
+      });
+      it('should not mark fileB as a link file', () => {
+        const fileA = 'fixtures/build-tree/not-link-file/file-a.js';
+        expect(results.tree[fileA].files)
+          .to.be.an('array')
+          .with.lengthOf(1);
+        const fileBDep = results.tree[fileA].files[0];
+        expect(fileBDep).to.not.have.property('isLink');
+      });
+    });
   });
 });

src/dependency-builder/detectives/detective-es6/index.js

@@ -26,6 +26,13 @@ module.exports = function (src) {
       dependencies[dependency].importSpecifiers = [importSpecifier];
     }
   };
+  const addExportedToImportSpecifier = (name) => {
+    Object.keys(dependencies).forEach((dependency) => {
+      if (!dependencies[dependency].importSpecifiers) return;
+      const specifier = dependencies[dependency].importSpecifiers.find(i => i.name === name);
+      if (specifier) specifier.exported = true;
+    });
+  };
 
   if (typeof src === 'undefined') {
     throw new Error('src not given');
@@ -60,13 +67,21 @@ module.exports = function (src) {
             node.specifiers.forEach((specifier) => {
               const specifierValue = {
                 isDefault: !specifier.local || specifier.local.name === 'default', // e.g. export { default as isArray } from './is-array';
-                name: specifier.exported.name
+                name: specifier.exported.name,
+                exported: true
               };
               addImportSpecifier(dependency, specifierValue);
             });
           }
+        } else if (node.specifiers && node.specifiers.length) {
+          node.specifiers.forEach((exportSpecifier) => {
+            addExportedToImportSpecifier(exportSpecifier.exported.name);
+          });
         }
         break;
+      case 'ExportDefaultDeclaration':
+        addExportedToImportSpecifier(node.declaration.name);
+        break;
       case 'CallExpression':
         if (node.callee.type === 'Import' && node.arguments.length && node.arguments[0].value) {
           addDependency(node.arguments[0].value);

src/dependency-builder/detectives/detective-es6/index.spec.js

@@ -146,5 +146,21 @@ describe('detective-es6', () => {
       expect(deps).to.have.property('foo');
       expect(deps.foo).to.not.have.property('importSpecifiers');
     });
+    it('should add "exported": true if the same variable has been imported and exported', () => {
+      const deps = detective('import { foo } from "foo"; export default foo;');
+      expect(deps).to.have.property('foo');
+      expect(deps.foo).to.have.property('importSpecifiers');
+      const importSpecifier = deps.foo.importSpecifiers[0];
+      expect(importSpecifier.name).to.equal('foo');
+      expect(importSpecifier.exported).to.be.true;
+    });
+    it('should not add "exported" property if the variable has been imported but not exported', () => {
+      const deps = detective('import { foo } from "foo";');
+      expect(deps).to.have.property('foo');
+      expect(deps.foo).to.have.property('importSpecifiers');
+      const importSpecifier = deps.foo.importSpecifiers[0];
+      expect(importSpecifier.name).to.equal('foo');
+      expect(importSpecifier).to.not.have.property('exported');
+    });
   });
 });

src/dependency-builder/detectives/detective-stylable/index.js

@@ -25,7 +25,8 @@ module.exports = function (src, options = {}) {
     if (!stNamed) return;
     const specifierValue = {
       isDefault: false, // @todo,
-      name: stNamed.value
+      name: stNamed.value,
+      exported: true
     };
     addImportSpecifier(stFromValue, specifierValue);
   });

src/dependency-builder/detectives/detective-typescript/index.js

@@ -30,6 +30,12 @@ module.exports = function (src, options = {}) {
       dependencies[dependency].importSpecifiers = [importSpecifier];
     }
   };
+  const addExportedToImportSpecifier = (name) => {
+    Object.keys(dependencies).forEach((dependency) => {
+      const specifier = dependencies[dependency].importSpecifiers.find(i => i.name === name);
+      if (specifier) specifier.exported = true;
+    });
+  };
 
   if (typeof src === 'undefined') {
     throw new Error('src not given');
@@ -59,8 +65,15 @@ module.exports = function (src, options = {}) {
       case 'ExportAllDeclaration':
         if (node.source && node.source.value) {
           addDependency(node.source.value);
+        } else if (node.specifiers && node.specifiers.length) {
+          node.specifiers.forEach((exportSpecifier) => {
+            addExportedToImportSpecifier(exportSpecifier.exported.name);
+          });
         }
         break;
+      case 'ExportDefaultDeclaration':
+        addExportedToImportSpecifier(node.declaration.name);
+        break;
       case 'TSExternalModuleReference':
         if (node.expression && node.expression.value) {
           addDependency(node.expression.value);

src/dependency-builder/path-map.js

@@ -78,7 +78,7 @@ function findTheRealDependency(
     visitedFiles.push(currentPathMap.file);
     const currentRealDep: ?PathMapDependency = currentPathMap.dependencies.find((dep) => {
       if (!dep.importSpecifiers) return false;
-      return dep.importSpecifiers.find(depSpecifier => depSpecifier.name === specifier.name);
+      return dep.importSpecifiers.find(depSpecifier => depSpecifier.name === specifier.name && depSpecifier.exported);
     });
     if (!currentRealDep) {
       // the currentRealDep is not the real dependency, return the last real we found

src/dependency-builder/types/dependency-tree-type.js

@@ -7,7 +7,8 @@
  */
 export type Specifier = {
   isDefault: boolean,
-  name: string
+  name: string,
+  exported?: boolean
 };
 
 /**