Skip to content

Commit

Permalink
resolve #517, when a nested dependency is imported directly, re-link …
Browse files Browse the repository at this point in the history
…all its dependents
  • Loading branch information
davidfirst committed Jan 2, 2018
1 parent f59def9 commit 9c95b08
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 32 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]

- when a nested dependency is imported directly, re-link all its dependents
- move `save-dependencies-as-components` flag from `bit import` command to be configurable in consumer bit.json

## [0.12.0-ext.10] - 2017-12-27
Expand Down
8 changes: 5 additions & 3 deletions e2e/commands/import.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -1448,8 +1448,7 @@ describe('bit import', function () {
});
});

// @todo: fix this case. currently it does change it in the bit.map, but the link on the filesystem still direct to the .dependency dir
describe.skip('import component is-type as a dependency of is-string and then import is-type directly', () => {
describe('import component is-type as a dependency of is-string and then import is-type directly', () => {
let localConsumerFiles;
before(() => {
helper.setNewLocalAndRemoteScopes();
Expand Down Expand Up @@ -1478,10 +1477,13 @@ describe('bit import', function () {
expect(bitMap[`${helper.remoteScope}/utils/[email protected]`].origin).to.equal('IMPORTED');
});
it('should not break the is-string component', () => {
const isTypeFixtureV2 = "module.exports = function isType() { return 'got is-type v2'; };";
helper.createComponent(path.join('components', 'utils', 'is-type'), 'is-type.js', isTypeFixtureV2);

const appJsFixture = "const isString = require('./components/utils/is-string'); console.log(isString());";
fs.outputFileSync(path.join(helper.localScopePath, 'app.js'), appJsFixture);
const result = helper.runCmd('node app.js');
expect(result.trim()).to.equal('got is-type and got is-string');
expect(result.trim()).to.equal('got is-type v2 and got is-string');
});
});

Expand Down
19 changes: 19 additions & 0 deletions e2e/flows/import-from-bit-hub.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,23 @@ describe('importing bit components from bitsrc.io', function () {
});
});
});
describe('importing a component as a dependency of other component and then importing it directly', () => {
before(() => {
helper.reInitLocalScope();
helper.runCmd('bit import david.tests/utils/is-string'); // is-string imports is-type as a dependency
helper.runCmd('bit import david.tests/utils/is-type'); // import is-type directly
});
describe('changing the directly imported dependency component', () => {
before(() => {
const isTypeFixtureV2 = "module.exports = function isType() { return 'got is-type v2'; };";
helper.createComponent(path.join('components', 'utils', 'is-type'), 'is-type.js', isTypeFixtureV2);
});
it('should affect its dependent', () => {
const appJsFixture = "const isString = require('./components/utils/is-string'); console.log(isString());";
fs.outputFileSync(path.join(helper.localScopePath, 'app.js'), appJsFixture);
const result = helper.runCmd('node app.js');
expect(result.trim()).to.equal('got is-type v2 and got is-string');
});
});
});
});
10 changes: 7 additions & 3 deletions src/consumer/bit-map/bit-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ export default class BitMap {
return new BitMap(dirPath, mapPath, components, version);
}

getAllComponents(origin?: ComponentOrigin): BitMapComponents {
getAllComponents(origin?: ComponentOrigin | ComponentOrigin[]): BitMapComponents {
if (!origin) return this.components;
const isOriginMatch = component => component.origin === origin;
return R.filter(isOriginMatch, this.components);
const isOriginMatchArray = component => origin.includes(component.origin);
const filter = Array.isArray(origin) ? isOriginMatchArray : isOriginMatch;
return R.filter(filter, this.components);
}

getAuthoredExportedComponents(): BitId[] {
Expand Down Expand Up @@ -299,7 +301,9 @@ export default class BitMap {
return;
}
if (olderComponentsIds.length > 1) {
throw new Error(`Your ${BIT_MAP} file has more than one version of ${id.toStringWithoutScopeAndVersion()} and they
throw new Error(`Your ${
BIT_MAP
} file has more than one version of ${id.toStringWithoutScopeAndVersion()} and they
are authored or imported. This scenario is not supported`);
}
const olderComponentId = olderComponentsIds[0];
Expand Down
38 changes: 25 additions & 13 deletions src/consumer/component/consumer-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
DEFAULT_REGISTRY_DOMAIN_PREFIX,
CFG_REGISTRY_DOMAIN_PREFIX
} from '../../constants';
import ComponentWithDependencies from '../../scope/component-dependencies';

export type RelativePath = {
sourceRelativePath: string,
Expand Down Expand Up @@ -98,6 +99,7 @@ export default class Component {
writtenPath: ?string; // needed for generate links
dependenciesSavedAsComponents: ?boolean = true; // otherwise they're saved as npm packages
originallySharedDir: ?string; // needed to reduce a potentially long path that was used by the author
_wasOriginallySharedDirStripped: ?boolean;
_writeDistsFiles: ?boolean = true;
_areDistsInsideComponentDir: ?boolean = true;
isolatedEnvironment: IsolatedEnvironment;
Expand Down Expand Up @@ -466,6 +468,7 @@ export default class Component {
* imports it. NESTED and AUTHORED components are written as is.
*/
stripOriginallySharedDir(bitMap: BitMap): void {
if (this._wasOriginallySharedDirStripped) return;
this.setOriginallySharedDir();
const originallySharedDir = this.originallySharedDir;
const pathWithoutSharedDir = (pathStr, sharedDir, isLinuxFormat) => {
Expand Down Expand Up @@ -500,6 +503,7 @@ export default class Component {
}
});
});
this._wasOriginallySharedDirStripped = true;
}

updateDistsPerConsumerBitJson(consumer: Consumer, componentMap: ComponentMap): void {
Expand Down Expand Up @@ -595,20 +599,13 @@ export default class Component {
if (origin === COMPONENT_ORIGINS.IMPORTED && componentMap.origin === COMPONENT_ORIGINS.NESTED) {
// when a user imports a component that was a dependency before, write the component directly into the components
// directory for an easy access/change. Then, remove the current record from bit.map and add an updated one.
await this._writeToComponentDir({
bitDir: calculatedBitDir,
withBitJson,
withPackageJson,
driver,
force,
writeBitDependencies,
dependencies,
deleteBitDirContent
});
// todo: remove from the file system
const oldLocation = path.join(consumerPath, componentMap.rootDir);
logger.debug(
`deleting the old directory of a component at ${oldLocation}, the new directory is ${calculatedBitDir}`
);
fs.removeSync(oldLocation);
bitMap.removeComponent(this.id.toString());
this._addComponentToBitMap(bitMap, calculatedBitDir, origin, parent);
return this;
componentMap = this._addComponentToBitMap(bitMap, calculatedBitDir, origin, parent);
}
logger.debug('component is in bit.map, write the files according to bit.map');
this.updateDistsPerConsumerBitJson(consumer, componentMap);
Expand Down Expand Up @@ -989,6 +986,21 @@ export default class Component {
this.originallySharedDir = sharedStart.substring(0, lastPathSeparator);
}

async toComponentWithDependencies(bitMap: BitMap, consumer: Consumer): Promise<ComponentWithDependencies> {
const getDependencies = () => {
return this.dependencies.map((dependency) => {
if (bitMap.isExistWithSameVersion(dependency.id)) {
return consumer.loadComponent(dependency.id);
}
// when dependencies are imported as npm packages, they are not in bit.map
this.dependenciesSavedAsComponents = false;
return consumer.scope.loadComponent(dependency.id, false);
});
};
const dependencies = await Promise.all(getDependencies());
return new ComponentWithDependencies({ component: this, dependencies });
}

static fromObject(object: Object): Component {
const {
name,
Expand Down
2 changes: 2 additions & 0 deletions src/consumer/component/link-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ async function writeDependencyLinks(
);
return Promise.resolve();
}
// it must be IMPORTED. We don't pass NESTED to this function
logger.debug(`writeDependencyLinks, generating links for ${componentWithDeps.component.id}`);
componentWithDeps.component.stripOriginallySharedDir(bitMap);

const directLinksP = componentLinks(componentWithDeps.dependencies, componentWithDeps.component, componentMap);

Expand Down
31 changes: 19 additions & 12 deletions src/consumer/consumer.js
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,27 @@ export default class Consumer {
? [...writtenComponents, ...R.flatten(writtenDependencies)]
: writtenComponents;
this.linkComponents(allComponents, bitMap);
await this.reLinkDirectlyImportedDependencies(writtenComponents, bitMap);
return allComponents;
}

/**
* an IMPORTED component might be NESTED before.
* find those components and re-link all their dependents
*/
async reLinkDirectlyImportedDependencies(potentialDependencies: Component[], bitMap: BitMap): void {
const fsComponents = bitMap.getAllComponents([COMPONENT_ORIGINS.IMPORTED, COMPONENT_ORIGINS.AUTHORED]);
const fsComponentsIds = Object.keys(fsComponents).map(component => BitId.parse(component));
const potentialDependenciesIds = potentialDependencies.map(c => c.id);
const components = await this.scope.findDirectDependentComponents(fsComponentsIds, potentialDependenciesIds);
if (!components.length) return;
const componentsWithDependencies = await Promise.all(
components.map(component => component.toComponentWithDependencies(bitMap, this))
);
await linkGenerator.writeDependencyLinks(componentsWithDependencies, bitMap, this, false);
this.linkComponents(components, bitMap);
}

moveExistingComponent(bitMap: BitMap, component: Component, oldPath: string, newPath: string) {
if (fs.existsSync(newPath)) {
throw new Error(
Expand Down Expand Up @@ -966,18 +984,7 @@ export default class Consumer {
}

async writeLinksInDist(component: Component, componentMap, bitMap: BitMap) {
const getDependencies = () => {
return component.dependencies.map((dependency) => {
if (bitMap.isExistWithSameVersion(dependency.id)) {
return this.loadComponent(dependency.id);
}
// when dependencies are imported as npm packages, they are not in bit.map
component.dependenciesSavedAsComponents = false;
return this.scope.loadComponent(dependency.id, false);
});
};
const dependencies = await Promise.all(getDependencies());
const componentWithDeps = new ComponentWithDependencies({ component, dependencies });
const componentWithDeps = await component.toComponentWithDependencies(bitMap, this);
await linkGenerator.writeDependencyLinks([componentWithDeps], bitMap, this, false);
const newMainFile = pathNormalizeToLinux(component.calculateMainDistFile());
await component.updatePackageJsonAttribute(this, componentMap.rootDir, 'main', newMainFile);
Expand Down
35 changes: 34 additions & 1 deletion src/scope/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import logger from '../logger/logger';
import componentResolver from '../component-resolver';
import ComponentsList from '../consumer/component/components-list';
import { RemovedObjects } from './component-remove';
import Component from '../consumer/component/consumer-component';

const removeNils = R.reject(R.isNil);
const pathHasScope = pathHas([OBJECTS_DIR, BIT_HIDDEN_DIR]);
Expand Down Expand Up @@ -852,7 +853,7 @@ export default class Scope {

if (!R.isEmpty(dependencies)) dependentBits[bitId.toStringWithoutVersion()] = R.uniq(dependencies);
});
return await Promise.resolve(dependentBits);
return Promise.resolve(dependentBits);
}

/**
Expand Down Expand Up @@ -946,6 +947,21 @@ export default class Scope {
return this.loadRemoteComponent(id);
}

/**
* load components from the model and return them as ComponentVersion array.
* if a component is not available locally, it'll just ignore it without throwing any error.
*/
async loadLocalComponents(ids: BitId[]): Promise<ComponentVersion[]> {
const componentsObjects = await this.sources.getMany(ids);
const components = componentsObjects.map((componentObject) => {
const component = componentObject.component;
if (!component) return null;
const version = componentObject.id.hasVersion() ? componentObject.id.version : component.latest();
return component.toComponentVersion(version);
});
return removeNils(components);
}

loadComponentLogs(id: BitId): Promise<{ [number]: { message: string, date: string, hash: string } }> {
return this.sources.get(id).then((componentModel) => {
if (!componentModel) throw new ComponentNotFound(id.toString());
Expand Down Expand Up @@ -1129,6 +1145,23 @@ export default class Scope {
return updatedComponents;
}

/**
* find the components in componentsPool which one of their dependencies include in potentialDependencies
*/
async findDirectDependentComponents(componentsPool: BitId[], potentialDependencies: BitId[]): Promise<Component[]> {
const componentsVersions = await this.loadLocalComponents(componentsPool);
const potentialDependenciesWithoutVersions = potentialDependencies.map(id => id.toStringWithoutVersion());
const dependentsP = componentsVersions.map(async (componentVersion: ComponentVersion) => {
const component = await componentVersion.getVersion(this.objects);
const found = component.dependencies.find(dependency =>
potentialDependenciesWithoutVersions.includes(dependency.id.toStringWithoutVersion())
);
return found ? componentVersion.toConsumer(this.objects) : null;
});
const dependents = await Promise.all(dependentsP);
return removeNils(dependents);
}

async runComponentSpecs({
bitId,
consumer,
Expand Down

0 comments on commit 9c95b08

Please sign in to comment.