Skip to content

Commit

Permalink
Introduce new flags "--dependents" and "--dependencies" for "bit impo…
Browse files Browse the repository at this point in the history
…rt" (#1927)

* implements #1885, introduce new flags "--dependents" and "--dependencies" for "bit import" to import them all directly

* fix a case when importing a component with --dependent flag and the component has the same dependent with different versions to import only the highest version

* fix the graph to include all versions and not only the latest
  • Loading branch information
davidfirst authored Aug 15, 2019
1 parent 21a1cfa commit 0752c30
Show file tree
Hide file tree
Showing 8 changed files with 294 additions and 53 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [unreleased]

- [#1885](https://github.com/teambit/bit/issues/1885) introduce new flags `--dependents` and `--dependencies` for `bit import` to import them all directly
- [#1924](https://github.com/teambit/bit/issues/1924) avoid generating old dists files when using capsule compilers

## [[14.2.4] - 2019-08-13](https://github.com/teambit/bit/releases/tag/v14.2.4)
Expand Down
105 changes: 105 additions & 0 deletions e2e/commands/import.e2e.1.js
Original file line number Diff line number Diff line change
Expand Up @@ -2237,4 +2237,109 @@ console.log(barFoo.default());`;
});
});
});
describe('import with --dependencies and --dependents flags', () => {
let scopeBeforeImport;
before(() => {
helper.setNewLocalAndRemoteScopes();
helper.populateWorkspaceWithComponents();
helper.createFile('utils', 'bar-dep.js');
helper.createFile('bar', 'foo2.js', 'require("../utils/bar-dep");');
helper.addComponent('utils/bar-dep.js');
helper.addComponent('bar/foo2.js', { i: 'bar/foo2' });
helper.tagAllComponents();
helper.exportAllComponents();
helper.reInitLocalScope();
helper.addRemoteScope();
scopeBeforeImport = helper.cloneLocalScope();
});
describe('import with --dependencies flag', () => {
before(() => {
helper.importComponent('bar/* --dependencies');
});
it('should import directly (not nested) all dependencies', () => {
const bitMap = helper.readBitMap();
expect(bitMap)
.to.have.property(`${helper.remoteScope}/utils/[email protected]`)
.that.has.property('origin')
.equal('IMPORTED');
expect(bitMap)
.to.have.property(`${helper.remoteScope}/utils/[email protected]`)
.that.has.property('origin')
.equal('IMPORTED');
expect(bitMap)
.to.have.property(`${helper.remoteScope}/[email protected]`)
.that.has.property('origin')
.equal('IMPORTED');
});
});
describe('import with --dependents flag', () => {
let output;
before(() => {
helper.getClonedLocalScope(scopeBeforeImport);
output = helper.importComponent('utils/is-type --dependents');
});
it('should import all dependents', () => {
expect(output).to.have.string('successfully imported 3 components');
expect(output).to.have.string(`${helper.remoteScope}/utils/is-string`);
expect(output).to.have.string(`${helper.remoteScope}/bar/foo`);
});
it('bit list should show them all', () => {
const list = helper.listLocalScope();
expect(list).to.have.string(`${helper.remoteScope}/utils/is-type`);
expect(list).to.have.string(`${helper.remoteScope}/utils/is-string`);
expect(list).to.have.string(`${helper.remoteScope}/bar/foo`);
});
});
});
// is-type has bar/[email protected] as an indirect dependent and bar/[email protected] as a direct dependent
describe('component with different versions of the same dependent', () => {
before(() => {
helper.setNewLocalAndRemoteScopes();
helper.populateWorkspaceWithComponents();
helper.tagAllComponents();
helper.createComponentBarFoo("require('../utils/is-type.js')");
helper.tagAllComponents();
helper.exportAllComponents();
helper.reInitLocalScope();
helper.addRemoteScope();
});
it('bit show of the remote scope should show both versions of the dependent', () => {
const show = helper.showComponentParsed(`${helper.remoteScope}/utils/is-type --remote --dependents`);
expect(show.dependentsInfo).to.have.lengthOf(3);
const barFooV1 = show.dependentsInfo.find(d => d.id.name === 'bar/foo' && d.id.version === '0.0.1');
const barFooV2 = show.dependentsInfo.find(d => d.id.name === 'bar/foo' && d.id.version === '0.0.2');
expect(barFooV1).to.not.be.undefined;
expect(barFooV2).to.not.be.undefined;
expect(barFooV1)
.to.have.property('depth')
.that.equals(2);
expect(barFooV2)
.to.have.property('depth')
.that.equals(1);
});
describe('import the component with "--dependents" flag', () => {
before(() => {
helper.importComponent('utils/is-type --dependents');
});
it('should import the dependent only once and with the highest version', () => {
const bitMap = helper.readBitMap();
expect(bitMap).to.have.property(`${helper.remoteScope}/bar/[email protected]`);
expect(bitMap).to.not.have.property(`${helper.remoteScope}/bar/[email protected]`);
});
it('bit show of the local scope show both versions of the dependent', () => {
const show = helper.showComponentParsed(`${helper.remoteScope}/utils/is-type --dependents`);
expect(show.dependentsInfo).to.have.lengthOf(3);
const barFooV1 = show.dependentsInfo.find(d => d.id.name === 'bar/foo' && d.id.version === '0.0.1');
const barFooV2 = show.dependentsInfo.find(d => d.id.name === 'bar/foo' && d.id.version === '0.0.2');
expect(barFooV1).to.not.be.undefined;
expect(barFooV2).to.not.be.undefined;
expect(barFooV1)
.to.have.property('depth')
.that.equals(2);
expect(barFooV2)
.to.have.property('depth')
.that.equals(1);
});
});
});
});
5 changes: 1 addition & 4 deletions src/api/consumer/lib/get-consumer-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { loadConsumer, Consumer } from '../../../consumer';
import NothingToCompareTo from './exceptions/nothing-to-compare-to';
import DependencyGraph from '../../../scope/graph/scope-graph';
import type { DependenciesInfo } from '../../../scope/graph/scope-graph';
import ComponentsList from '../../../consumer/component/components-list';

export default (async function getConsumerBit({
id,
Expand All @@ -29,9 +28,7 @@ export default (async function getConsumerBit({
let dependenciesInfo: DependenciesInfo[] = [];
let dependentsInfo: DependenciesInfo[] = [];
if (showDependents || showDependencies) {
const componentsList = new ComponentsList(consumer);
const allComponents = await componentsList.getFromFileSystem();
const graph = DependencyGraph.buildGraphFromComponents(allComponents);
const graph = await DependencyGraph.buildGraphFromWorkspace(consumer);
const dependencyGraph = new DependencyGraph(graph);
const componentGraph = dependencyGraph.getSubGraphOfConnectedComponents(component.id);
const componentDepGraph = new DependencyGraph(componentGraph);
Expand Down
16 changes: 12 additions & 4 deletions src/cli/commands/public-cmds/import-cmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ export default class Import extends Command {
'm',
'merge [strategy]',
'merge local changes with the imported version. strategy should be "theirs", "ours" or "manual"'
]
],
['', 'dependencies', 'EXPERIMENTAL. import all dependencies and write them to the workspace'],
['', 'dependents', 'EXPERIMENTAL. import component dependents to allow auto-tag updating them upon tag']
];
loader = true;
migration = true;
Expand All @@ -73,7 +75,9 @@ export default class Import extends Command {
conf,
skipNpmInstall = false,
ignorePackageJson = false,
merge
merge,
dependencies = false,
dependents = false
}: {
tester?: boolean,
compiler?: boolean,
Expand All @@ -88,7 +92,9 @@ export default class Import extends Command {
conf?: string,
skipNpmInstall?: boolean,
ignorePackageJson?: boolean,
merge?: MergeStrategy
merge?: MergeStrategy,
dependencies?: boolean,
dependents?: boolean
},
packageManagerArgs: string[]
): Promise<any> {
Expand Down Expand Up @@ -127,7 +133,9 @@ export default class Import extends Command {
writeDists: !ignoreDist,
writeConfig: !!conf,
installNpmPackages: !skipNpmInstall,
writePackageJson: !ignorePackageJson
writePackageJson: !ignorePackageJson,
importDependenciesDirectly: dependencies,
importDependents: dependents
};
// From the CLI you can pass the conf as path or just --conf (which will later translate to the default eject conf folder)
if (typeof conf === 'string') {
Expand Down
88 changes: 77 additions & 11 deletions src/consumer/component-ops/import-components.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/** @flow */
import R from 'ramda';
import semver from 'semver';
import chalk from 'chalk';
import { NothingToImport } from '../exceptions';
import { BitId } from '../../bit-id';
import { BitId, BitIds } from '../../bit-id';
import Component from '../component';
import { Consumer } from '../../consumer';
import { ComponentWithDependencies, Scope } from '../../scope';
Expand All @@ -19,6 +20,9 @@ import ManyComponentsWriter from './many-components-writer';
import { COMPONENT_ORIGINS } from '../../constants';
import hasWildcard from '../../utils/string/has-wildcard';
import { getRemoteBitIdsByWildcards } from '../../api/consumer/lib/list-scope';
import { getScopeRemotes } from '../../scope/scope-remotes';
import Remotes from '../../remotes/remotes';
import DependencyGraph from '../../scope/graph/scope-graph';

export type ImportOptions = {
ids: string[], // array might be empty
Expand All @@ -34,7 +38,9 @@ export type ImportOptions = {
override: boolean, // default: false
installNpmPackages: boolean, // default: true
objectsOnly: boolean, // default: false
saveDependenciesAsComponents?: boolean // default: false
saveDependenciesAsComponents?: boolean, // default: false,
importDependenciesDirectly?: boolean, // default: false, normally it imports them as packages or nested, not as imported
importDependents?: boolean // default: false,
};
type ComponentMergeStatus = {
componentWithDependencies: ComponentWithDependencies,
Expand Down Expand Up @@ -76,7 +82,7 @@ export default class ImportComponents {

async importSpecificComponents(): ImportResult {
logger.debug(`importSpecificComponents, Ids: ${this.options.ids.join(', ')}`);
const bitIds = await this._getBitIds();
const bitIds: BitIds = await this._getBitIds();
const beforeImportVersions = await this._getCurrentVersions(bitIds);
await this._throwForPotentialIssues(bitIds);
const componentsWithDependencies = await this.consumer.importComponents(
Expand All @@ -85,12 +91,31 @@ export default class ImportComponents {
this.options.saveDependenciesAsComponents
);
await this._throwForModifiedOrNewDependencies(componentsWithDependencies);
await this._writeToFileSystem(componentsWithDependencies);
const componentsWithDependenciesFiltered = this._filterComponentsWithLowerVersions(componentsWithDependencies);
await this._writeToFileSystem(componentsWithDependenciesFiltered);
const importDetails = await this._getImportDetails(beforeImportVersions, componentsWithDependencies);
return { dependencies: componentsWithDependencies, importDetails };
return { dependencies: componentsWithDependenciesFiltered, importDetails };
}

async _getBitIds(): Promise<BitId[]> {
/**
* it can happen for example when importing a component with `--dependent` flag and the component has
* the same dependent with different versions. we only want the one with the higher version
*/
_filterComponentsWithLowerVersions(
componentsWithDependencies: ComponentWithDependencies[]
): ComponentWithDependencies[] {
return componentsWithDependencies.filter((comp) => {
const sameIdHigherVersion = componentsWithDependencies.find(
c =>
!c.component.id.isEqual(comp.component.id) &&
c.component.id.isEqualWithoutVersion(comp.component.id) &&
semver.gt(c.component.id.version, comp.component.id.version)
);
return !sameIdHigherVersion;
});
}

async _getBitIds(): Promise<BitIds> {
const bitIds: BitId[] = [];
await Promise.all(
this.options.ids.map(async (idStr: string) => {
Expand All @@ -103,7 +128,47 @@ export default class ImportComponents {
}
})
);
return bitIds;
if (this.options.importDependenciesDirectly || this.options.importDependents) {
const graphs = await this._getComponentsGraphs(bitIds);
if (this.options.importDependenciesDirectly) {
const dependenciesIds = this._getDependenciesFromGraph(bitIds, graphs);
bitIds.push(...dependenciesIds);
}
if (this.options.importDependents) {
const dependentsIds = this._getDependentsFromGraph(bitIds, graphs);
bitIds.push(...dependentsIds);
}
}
return BitIds.uniqFromArray(bitIds);
}

_getDependenciesFromGraph(bitIds: BitId[], graphs: DependencyGraph[]): BitId[] {
const dependencies = bitIds.map((bitId) => {
const componentGraph = graphs.find(graph => graph.scopeName === bitId.scope);
if (!componentGraph) {
throw new Error(`unable to find a graph for ${bitId.toString()}`);
}
const dependenciesInfo = componentGraph.getDependenciesInfo(bitId);
return dependenciesInfo.map(d => d.id);
});
return R.flatten(dependencies);
}

_getDependentsFromGraph(bitIds: BitId[], graphs: DependencyGraph[]): BitId[] {
const dependents = bitIds.map((bitId) => {
const componentGraph = graphs.find(graph => graph.scopeName === bitId.scope);
if (!componentGraph) {
throw new Error(`unable to find a graph for ${bitId.toString()}`);
}
const dependentsInfo = componentGraph.getDependentsInfo(bitId);
return dependentsInfo.map(d => d.id);
});
return R.flatten(dependents);
}

async _getComponentsGraphs(bitIds: BitId[]): Promise<DependencyGraph[]> {
const remotes: Remotes = await getScopeRemotes(this.consumer.scope);
return remotes.scopeGraphs(bitIds, this.consumer.scope);
}

async importAccordingToBitMap(): ImportResult {
Expand Down Expand Up @@ -158,7 +223,7 @@ export default class ImportComponents {
return { dependencies: componentsAndDependencies, importDetails };
}

async _getCurrentVersions(ids: BitId[]): ImportedVersions {
async _getCurrentVersions(ids: BitIds): ImportedVersions {
const versionsP = ids.map(async (id) => {
const modelComponent = await this.consumer.scope.getModelComponentIfExist(id);
const idStr = id.toStringWithoutVersion();
Expand Down Expand Up @@ -202,16 +267,17 @@ export default class ImportComponents {
return Promise.all(detailsP);
}

async _throwForPotentialIssues(ids: BitId[]): Promise<void> {
async _throwForPotentialIssues(ids: BitIds): Promise<void> {
await this._throwForModifiedOrNewComponents(ids);
this._throwForDifferentComponentWithSameName(ids);
}

async _throwForModifiedOrNewComponents(ids: BitId[]): Promise<void> {
async _throwForModifiedOrNewComponents(ids: BitIds): Promise<void> {
// the typical objectsOnly option is when a user cloned a project with components tagged to the source code, but
// doesn't have the model objects. in that case, calling getComponentStatusById() may return an error as it relies
// on the model objects when there are dependencies
if (this.options.override || this.options.objectsOnly || this.options.merge) return;
// $FlowFixMe BitIds is an array
const modifiedComponents = await filterAsync(ids, (id) => {
return this.consumer.getComponentStatusById(id).then(status => status.modified || status.newlyCreated);
});
Expand Down Expand Up @@ -240,7 +306,7 @@ export default class ImportComponents {
* If an imported component has scope+name equals to a local name, both will have the exact same
* hash and they'll override each other.
*/
_throwForDifferentComponentWithSameName(ids: BitId[]): void {
_throwForDifferentComponentWithSameName(ids: BitIds): void {
ids.forEach((id: BitId) => {
const existingId = this.consumer.getParsedIdIfExist(id.toStringWithoutVersion());
if (existingId && !existingId.hasScope()) {
Expand Down
2 changes: 1 addition & 1 deletion src/remotes/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default class Remote {
return this.connect().then(network => network.show(bitId));
}

graph(bitId?: BitId): Promise<DependencyGraph> {
graph(bitId?: ?BitId): Promise<DependencyGraph> {
return this.connect().then(network => network.graph(bitId));
}

Expand Down
Loading

0 comments on commit 0752c30

Please sign in to comment.