Skip to content

Commit

Permalink
fix(lane-merge), merge recovered components (#7760)
Browse files Browse the repository at this point in the history
Currently, if the checked out lane has a removed component and it is
merging a lane where in that lane the component has been recovered, it
won't merge the component, just skip it with a message saying the
component is removed.
This PR checks whether the other lane is ahead, in which case, we know
that the other lane recovered the removed component, and we bring it
back.
  • Loading branch information
davidfirst authored Aug 9, 2023
1 parent 1f76c51 commit b97c92f
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 5 deletions.
49 changes: 49 additions & 0 deletions e2e/harmony/recover.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,53 @@ describe('bit recover command', function () {
expect(removeData.config.removed).to.be.true;
});
});
describe('remove in one lane, recover in other lane, then merged to the first lane', () => {
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopes();
helper.command.createLane('lane-a');
helper.fixtures.populateComponents(2);
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();

helper.command.createLane('lane-b');
helper.command.snapAllComponentsWithoutBuild('--unmodified');
helper.command.export();

helper.command.switchLocalLane('lane-a', '-x');
helper.command.removeLaneComp('comp1');
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();

helper.command.switchLocalLane('lane-b', '-x');
helper.command.mergeLane('lane-a', '-x');
helper.command.recover(`${helper.scopes.remote}/comp1`);
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();
helper.command.switchLocalLane('lane-a', '-x');
helper.command.mergeLane('lane-b');
});
it('should bring back the previously removed component', () => {
const list = helper.command.listParsed();
expect(list).to.have.lengthOf(2);
helper.bitMap.expectToHaveId('comp1');
});
});

describe('remove in one lane, recover in an empty lane', () => {
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopes();
helper.command.createLane('lane-a');
helper.fixtures.populateComponents(2);
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();
helper.command.removeLaneComp('comp1');
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();
helper.command.createLane('lane-b');
});
// currently it threw version "0.0.0" of component onpp7beq-remote/comp1 was not found
it('should show a descriptive error', () => {
expect(() => helper.command.recover(`${helper.scopes.remote}/comp1`)).to.throw('unable to find the component');
});
});
});
12 changes: 11 additions & 1 deletion scopes/component/merging/merging.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,17 @@ export class MergingMain {
};
const currentComponent = await getCurrentComponent();
if (currentComponent.isRemoved()) {
return returnUnmerged(`component has been removed`, true);
// we made sure before that the component is not removed on the "other". so we have a few options:
// 1. other is ahead. in this case, other recovered the component. so we can continue with the merge.
// it is possible that it is diverged, in which case, still continue with the merge, and later on, the
// merge-config will show a config conflict of the remove aspect.
// 2. other is not ahead. in this case, just ignore this component, no point to merge it, we want it removed.
// 3. there are errors when calculating the divergeData, e.g. no snap in common. in such cases, we assume
// there are issues with this component, and is better not to merge it.
const divergeData = await getDivergeData({ repo, modelComponent, targetHead: otherLaneHead, throws: false });
if (divergeData.err || !divergeData.isTargetAhead()) {
return returnUnmerged(`component has been removed`, true);
}
}
const isModified = async () => {
const componentModificationStatus = await consumer.getComponentStatusById(currentComponent.id);
Expand Down
20 changes: 17 additions & 3 deletions scopes/component/remove/remove.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { ComponentID } from '@teambit/component-id';
import { BitError } from '@teambit/bit-error';
import deleteComponentsFiles from '@teambit/legacy/dist/consumer/component-ops/delete-component-files';
import ComponentAspect, { Component, ComponentMain } from '@teambit/component';
import { VersionNotFound } from '@teambit/legacy/dist/scope/exceptions';
import { removeComponentsFromNodeModules } from '@teambit/legacy/dist/consumer/component/package-json-utils';
import { RemoveCmd } from './remove-cmd';
import { removeComponents } from './remove-components';
Expand Down Expand Up @@ -163,11 +164,24 @@ export class RemoveMain {
return true;
}
const compId = await this.workspace.scope.resolveComponentId(compIdStr);
const compFromScope = await this.workspace.scope.get(compId);
const currentLane = await this.workspace.getCurrentLaneObject();
const idOnLane = currentLane?.getComponent(compId._legacy);
const compIdWithPossibleVer = idOnLane ? compId.changeVersion(idOnLane.head.toString()) : compId;
let compFromScope: Component | undefined;
try {
compFromScope = await this.workspace.scope.get(compIdWithPossibleVer);
} catch (err: any) {
if (err instanceof VersionNotFound && err.version === '0.0.0') {
throw new BitError(
`unable to find the component ${compIdWithPossibleVer.toString()} in the current lane or main`
);
}
throw err;
}
if (compFromScope && this.isRemoved(compFromScope)) {
// case #2 and #3
await importComp(compId._legacy.toString());
await setAsRemovedFalse(compId);
await importComp(compIdWithPossibleVer._legacy.toString());
await setAsRemovedFalse(compIdWithPossibleVer);
return true;
}
// case #5
Expand Down
2 changes: 1 addition & 1 deletion src/scope/exceptions/version-not-found.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import chalk from 'chalk';
* @see VersionNotFoundOnFS for cases when the Version object is missing from the filesystem.
*/
export default class VersionNotFound extends BitError {
constructor(version: string, componentId: string) {
constructor(readonly version: string, componentId: string) {
super(`error: version "${chalk.bold(version)}" of component ${chalk.bold(componentId)} was not found.`);
}
}

0 comments on commit b97c92f

Please sign in to comment.