Skip to content
This repository was archived by the owner on Dec 4, 2022. It is now read-only.

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

Merged
merged 5 commits into from
May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions fixtures/build-tree/not-link-file/file-a.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import { varX } from './file-b';
3 changes: 3 additions & 0 deletions fixtures/build-tree/not-link-file/file-b.js
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 2 additions & 0 deletions fixtures/build-tree/not-link-file/file-c.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const varX = 4;
export { varX };
2 changes: 1 addition & 1 deletion fixtures/build-tree/tree-shaking-cycle/is-string.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
import { isString } from '.'; // cycle with ./index.js
export default function() {}
export default isString;
12 changes: 8 additions & 4 deletions fixtures/path-map.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
"importSpecifiers": [
{
"isDefault": true,
"name": "isString"
"name": "isString",
"exported": true
}
]
},
Expand All @@ -37,7 +38,8 @@
"importSpecifiers": [
{
"isDefault": true,
"name": "isArray"
"name": "isArray",
"exported": true
}
]
}
Expand All @@ -52,7 +54,8 @@
"importSpecifiers": [
{
"isDefault": true,
"name": "isString"
"name": "isString",
"exported": true
}
]
}
Expand All @@ -71,7 +74,8 @@
"importSpecifiers": [
{
"isDefault": true,
"name": "isArray"
"name": "isArray",
"exported": true
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
15 changes: 15 additions & 0 deletions src/dependency-builder/build-tree.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
});
17 changes: 16 additions & 1 deletion src/dependency-builder/detectives/detective-es6/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions src/dependency-builder/detectives/detective-es6/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
13 changes: 13 additions & 0 deletions src/dependency-builder/detectives/detective-typescript/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/dependency-builder/path-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/dependency-builder/types/dependency-tree-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
*/
export type Specifier = {
isDefault: boolean,
name: string
name: string,
exported?: boolean
};

/**
Expand Down