From 9c95b08f0673b43cb5dfa68e5cdef9a04a27bbc3 Mon Sep 17 00:00:00 2001 From: David First Date: Tue, 2 Jan 2018 14:38:21 -0500 Subject: [PATCH] resolve #517, when a nested dependency is imported directly, re-link all its dependents --- CHANGELOG.md | 1 + e2e/commands/import.e2e.js | 8 +++-- e2e/flows/import-from-bit-hub.e2e.js | 19 ++++++++++ src/consumer/bit-map/bit-map.js | 10 ++++-- src/consumer/component/consumer-component.js | 38 +++++++++++++------- src/consumer/component/link-generator.js | 2 ++ src/consumer/consumer.js | 31 +++++++++------- src/scope/scope.js | 35 +++++++++++++++++- 8 files changed, 112 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bbff3c813da..bb03bb5243d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/e2e/commands/import.e2e.js b/e2e/commands/import.e2e.js index 4402f81fe970..d4dc0a04a4ff 100644 --- a/e2e/commands/import.e2e.js +++ b/e2e/commands/import.e2e.js @@ -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(); @@ -1478,10 +1477,13 @@ describe('bit import', function () { expect(bitMap[`${helper.remoteScope}/utils/is-type@0.0.1`].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'); }); }); diff --git a/e2e/flows/import-from-bit-hub.e2e.js b/e2e/flows/import-from-bit-hub.e2e.js index 0e25bc420d55..fee6216e3f1f 100644 --- a/e2e/flows/import-from-bit-hub.e2e.js +++ b/e2e/flows/import-from-bit-hub.e2e.js @@ -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'); + }); + }); + }); }); diff --git a/src/consumer/bit-map/bit-map.js b/src/consumer/bit-map/bit-map.js index 5ed8a6095e35..15b6a880be5e 100644 --- a/src/consumer/bit-map/bit-map.js +++ b/src/consumer/bit-map/bit-map.js @@ -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[] { @@ -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]; diff --git a/src/consumer/component/consumer-component.js b/src/consumer/component/consumer-component.js index 7baac569e09d..69aa2025c558 100644 --- a/src/consumer/component/consumer-component.js +++ b/src/consumer/component/consumer-component.js @@ -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, @@ -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; @@ -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) => { @@ -500,6 +503,7 @@ export default class Component { } }); }); + this._wasOriginallySharedDirStripped = true; } updateDistsPerConsumerBitJson(consumer: Consumer, componentMap: ComponentMap): void { @@ -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); @@ -989,6 +986,21 @@ export default class Component { this.originallySharedDir = sharedStart.substring(0, lastPathSeparator); } + async toComponentWithDependencies(bitMap: BitMap, consumer: Consumer): Promise { + 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, diff --git a/src/consumer/component/link-generator.js b/src/consumer/component/link-generator.js index 0e2120681f78..c1bcbc4b43f0 100644 --- a/src/consumer/component/link-generator.js +++ b/src/consumer/component/link-generator.js @@ -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); diff --git a/src/consumer/consumer.js b/src/consumer/consumer.js index a67836c37dc2..baee82dbe704 100644 --- a/src/consumer/consumer.js +++ b/src/consumer/consumer.js @@ -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( @@ -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); diff --git a/src/scope/scope.js b/src/scope/scope.js index a5975616baaa..b6700b2406d8 100644 --- a/src/scope/scope.js +++ b/src/scope/scope.js @@ -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]); @@ -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); } /** @@ -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 { + 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()); @@ -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 { + 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,