Skip to content

Commit

Permalink
fix: avoid throwing ComponentNotFound when .bitmap has a non-exist ve…
Browse files Browse the repository at this point in the history 
…rsion on the scope (#6496)

Happens when an exported component has a local tag and `.bit` got deleted. The local scope doesn't have this local tag anymore and it never reached the remote. 
Previously, this use-case was handled by the out-of-sync algorithm, which synced the version according to the remote.
Since Harmony, a new load-hook was added before the out-of-sync logic, which imports components from remotes. This import threw ComponentNotFound in this case. 
This PR fixes it by running some of the out-of-sync before the on-load hook gets called.
  • Loading branch information
@davidfirst
davidfirst authored Oct 5, 2022
1 parent 35a38c5 commit bc03232
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 274 deletions.
13 changes: 4 additions & 9 deletions e2e/flows/out-of-sync-componets.e2e.3.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,23 +272,20 @@ describe('components that are not synced between the scope and the consumer', fu
helper.general.expectToThrow(() => helper.command.status(), err);
});
});
// @TODO: FIX ON HARMONY!
describe.skip('bit show', () => {
describe('bit show', () => {
it('should throw an error suggesting to import the components', () => {
const err = new ComponentsPendingImport();
helper.general.expectToThrow(() => helper.command.showComponent('bar/foo'), err);
});
});
// @TODO: FIX ON HARMONY!
describe.skip('bit tag', () => {
describe('bit tag', () => {
it('should throw an error suggesting to import the components', () => {
const err = new ComponentsPendingImport();
helper.general.expectToThrow(() => helper.command.tagAllWithoutBuild(), err);
});
});
});
// @TODO: FIX ON HARMONY!
describe.skip('when the remote component does not exist or does not have this missing version', () => {
describe('when the remote component does not exist or does not have this missing version', () => {
let scopeAfterV1;
let scopeOutOfSync;
before(() => {
Expand All @@ -313,9 +310,7 @@ describe('components that are not synced between the scope and the consumer', fu
});
it('should sync .bitmap according to the latest version of the scope', () => {
helper.command.expectStatusToBeClean();
const bitMap = helper.bitMap.read();
const newId = `${helper.scopes.remote}/bar/[email protected]`;
expect(bitMap).to.have.property(newId);
helper.bitMap.expectToHaveIdHarmony('bar/foo', '0.0.1', helper.scopes.remote);
});
});
describe('bit tag', () => {
Expand Down
3 changes: 1 addition & 2 deletions e2e/harmony/lanes/lanes.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1240,8 +1240,7 @@ describe('bit lane command', function () {
});
});
});
// @todo: fix!
describe.skip('head on the lane is not in the filesystem', () => {
describe('head on the lane is not in the filesystem', () => {
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopesHarmony();
helper.bitJsonc.setupDefault();
Expand Down
5 changes: 3 additions & 2 deletions scopes/workspace/workspace/workspace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -808,11 +808,12 @@ the following envs are used in this workspace: ${availableEnvs.join(', ')}`);
if (!lane) {
return;
}
this.logger.debug(`current lane ${laneId.toString()} is missing, importing it`);
this.logger.info(`current lane ${laneId.toString()} is missing, importing it`);
await this.scope.legacyScope.objects.writeObjectsToTheFS([lane]);
const scopeComponentsImporter = ScopeComponentsImporter.getInstance(this.scope.legacyScope);
const ids = BitIds.fromArray(lane.toBitIds().filter((id) => id.hasScope()));
await scopeComponentsImporter.importManyDeltaWithoutDeps(ids, true, lane);
await scopeComponentsImporter.importMany({ ids, lanes: lane ? [lane] : undefined });
await scopeComponentsImporter.importMany({ ids, lanes: [lane] });
}

/**
Expand Down
52 changes: 32 additions & 20 deletions src/consumer/component/component-loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import Consumer from '../consumer';
import { ComponentFsCache } from './component-fs-cache';
import { updateDependenciesVersions } from './dependencies/dependency-resolver';
import { DependenciesLoader } from './dependencies/dependency-resolver/dependencies-loader';
import ComponentMap from '../bit-map/component-map';

export type ComponentLoadOptions = {
loadDocs?: boolean;
Expand Down Expand Up @@ -143,7 +144,7 @@ export default class ComponentLoader {
removedComponents: Component[],
loadOpts?: ComponentLoadOptions
) {
const componentMap = this.consumer.bitMap.getComponent(id);
let componentMap = this.consumer.bitMap.getComponent(id);
if (componentMap.isRemoved()) {
const fromModel = await this.consumer.scope.getConsumerComponentIfExist(id);
if (!fromModel) {
Expand All @@ -161,7 +162,6 @@ export default class ComponentLoader {
removedComponents.push(fromModel);
return null;
}
const bitDir = path.join(this.consumer.getPath(), componentMap.rootDir);
let component: Component;
const handleError = (error) => {
if (throwOnFailure) throw error;
Expand All @@ -175,20 +175,25 @@ export default class ComponentLoader {
}
throw error;
};
const newId = await this._handleOutOfSyncScenarios(componentMap);
if (newId) {
componentMap = this.consumer.bitMap.getComponent(newId);
}
const updatedId = newId || id;

try {
component = await Component.loadFromFileSystem({
bitDir,
componentMap,
id,
id: updatedId,
consumer: this.consumer,
});
} catch (err: any) {
return handleError(err);
}
component.loadedFromFileSystem = true;
// reload component map as it may be changed after calling Component.loadFromFileSystem()
component.componentMap = this.consumer.bitMap.getComponent(id);
await this._handleOutOfSyncScenarios(component);
component.componentMap = this.consumer.bitMap.getComponent(updatedId);
await this._handleOutOfSyncWithDefaultScope(component);

const loadDependencies = async () => {
await this.invalidateDependenciesCacheIfNeeded();
Expand Down Expand Up @@ -226,11 +231,10 @@ export default class ComponentLoader {
});
}

private async _handleOutOfSyncScenarios(component: Component) {
const { componentFromModel, componentMap } = component;
// @ts-ignore componentMap is set here
private async _handleOutOfSyncScenarios(componentMap: ComponentMap): Promise<BitId | undefined> {
const currentId: BitId = componentMap.id;
let newId: BitId | null | undefined;
const componentFromModel = await this.consumer.loadComponentFromModelIfExist(currentId);
let newId: BitId | undefined;
if (componentFromModel && !currentId.hasVersion()) {
// component is in the scope but .bitmap doesn't have version, sync .bitmap with the scope data
newId = currentId.changeVersion(componentFromModel.version);
Expand All @@ -248,30 +252,38 @@ export default class ComponentLoader {
// latest version from the scope
await this._throwPendingImportIfNeeded(currentId);
newId = currentId.changeVersion(modelComponent.latest());
component.componentFromModel = await this.consumer.loadComponentFromModelIfExist(newId);
} else if (!currentId.hasScope()) {
// the scope doesn't have this component and .bitmap doesn't have scope, assume it's new
newId = currentId.changeVersion(undefined);
}
}
if (!componentFromModel && !currentId.hasVersion() && component.defaultScope) {

if (newId) {
this.consumer.bitMap.updateComponentId(newId);
}
return newId;
}

private async _handleOutOfSyncWithDefaultScope(component: Component) {
const { componentFromModel, componentMap } = component;
// @ts-ignore componentMap is set here
const currentId: BitId = componentMap.id;
if (!componentFromModel && !currentId.hasVersion()) {
// for Harmony, we know ahead the defaultScope, so even then .bitmap shows it as new and
// there is nothing in the scope, we can check if there is a component with the same
// default-scope in the objects
const modelComponent = await this.consumer.scope.getModelComponentIfExist(
currentId.changeScope(component.defaultScope)
);
if (modelComponent) {
newId = currentId.changeVersion(modelComponent.latest()).changeScope(modelComponent.scope);
const newId = currentId.changeVersion(modelComponent.latest()).changeScope(modelComponent.scope);
component.componentFromModel = await this.consumer.loadComponentFromModelIfExist(newId);
}
}

if (newId) {
component.version = newId.version;
component.scope = newId.scope;
this.consumer.bitMap.updateComponentId(newId);
component.componentMap = this.consumer.bitMap.getComponent(newId);
component.version = newId.version;
component.scope = newId.scope;
this.consumer.bitMap.updateComponentId(newId);
component.componentMap = this.consumer.bitMap.getComponent(newId);
}
}
}

Expand Down
18 changes: 5 additions & 13 deletions src/consumer/component/consumer-component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import ComponentWithDependencies from '../../scope/component-dependencies';
import { ScopeListItem } from '../../scope/models/model-component';
import Version, { Log } from '../../scope/models/version';
import { pathNormalizeToLinux } from '../../utils';
import { PathLinux, PathOsBased, PathOsBasedAbsolute, PathOsBasedRelative } from '../../utils/path';
import { PathLinux, PathOsBased, PathOsBasedRelative } from '../../utils/path';
import ComponentMap from '../bit-map/component-map';
import { IgnoredDirectory } from '../component-ops/add-components/exceptions/ignored-directory';
import ComponentsPendingImport from '../component-ops/exceptions/components-pending-import';
Expand Down Expand Up @@ -496,19 +496,15 @@ export default class Component {
return this.fromObject(object);
}

// eslint-disable-next-line complexity
static async loadFromFileSystem({
bitDir,
componentMap,
id,
consumer,
}: {
bitDir: PathOsBasedAbsolute;
componentMap: ComponentMap;
id: BitId;
consumer: Consumer;
}): Promise<Component> {
const consumerPath = consumer.getPath();
const workspaceConfig: ILegacyWorkspaceConfig = consumer.config;
const modelComponent = await consumer.scope.getModelComponentIfExist(id);
const componentFromModel = await consumer.loadComponentFromModelIfExist(id);
Expand All @@ -518,20 +514,16 @@ export default class Component {
if (!inScopeWithAnyVersion) throw new ComponentsPendingImport();
}
const deprecated = componentFromModel ? componentFromModel.deprecated : false;
const componentDir = componentMap.getComponentDir();
if (!fs.existsSync(bitDir)) throw new ComponentNotFoundInPath(bitDir);
const compDirAbs = path.join(consumer.getPath(), componentMap.getComponentDir());
if (!fs.existsSync(compDirAbs)) throw new ComponentNotFoundInPath(compDirAbs);

// Load the base entry from the root dir in map file in case it was imported using -path
// Or created using bit create so we don't want all the path but only the relative one
// Check that bitDir isn't the same as consumer path to make sure we are not loading global stuff into component
// (like dependencies)
logger.trace(`consumer-component.loadFromFileSystem, start loading config ${id.toString()}`);
const componentConfig = await ComponentConfig.load({
consumer,
componentId: id,
componentDir,
workspaceDir: consumerPath,
workspaceConfig,
});
logger.trace(`consumer-component.loadFromFileSystem, finish loading config ${id.toString()}`);
// by default, imported components are not written with bit.json file.
Expand All @@ -556,7 +548,7 @@ export default class Component {
);
const packageJsonFile = (componentConfig && componentConfig.packageJsonFile) || undefined;
const packageJsonChangedProps = componentFromModel ? componentFromModel.packageJsonChangedProps : undefined;
const files = await getLoadedFilesHarmony(consumer, componentMap, id, bitDir);
const files = await getLoadedFiles(consumer, componentMap, id, compDirAbs);
const docsP = _getDocsForFiles(files, consumer.componentFsCache);
const docs = await Promise.all(docsP);
const flattenedDocs = docs ? R.flatten(docs) : [];
Expand Down Expand Up @@ -593,7 +585,7 @@ export default class Component {
}
}

async function getLoadedFilesHarmony(
async function getLoadedFiles(
consumer: Consumer,
componentMap: ComponentMap,
id: BitId,
Expand Down
Loading

0 comments on commit bc03232

Please sign in to comment.