From 613ca85a1beddee650fb39362801b481edd715fa Mon Sep 17 00:00:00 2001 From: David First Date: Tue, 12 Mar 2024 17:23:06 -0400 Subject: [PATCH 1/2] fix(lane-merge), avoid removing dependencies that were set explicitly before --- e2e/harmony/merge-config.e2e.ts | 33 ++++++++++++++++++++ scopes/workspace/workspace/aspects-merger.ts | 26 +++++++++++++++ src/e2e-helper/e2e-command-helper.ts | 1 + 3 files changed, 60 insertions(+) diff --git a/e2e/harmony/merge-config.e2e.ts b/e2e/harmony/merge-config.e2e.ts index 6b7c054ebe2f..6f89fb02aa13 100644 --- a/e2e/harmony/merge-config.e2e.ts +++ b/e2e/harmony/merge-config.e2e.ts @@ -621,4 +621,37 @@ describe('merge config scenarios', function () { }); } ); + describe('diverge with merge-able auto-detected dependencies config and pre-config explicitly set', () => { + let mainBeforeDiverge: string; + before(() => { + helper.scopeHelper.setNewLocalAndRemoteScopes(); + helper.fixtures.populateComponents(1); + helper.fs.outputFile('comp1/index.js', `import R from 'ramda';`); + helper.npm.addFakeNpmPackage('ramda', '0.0.19'); + helper.npm.addFakeNpmPackage('lodash', '4.2.4'); + helper.command.dependenciesSet('comp1', 'lodash@4.2.4'); + helper.command.tagAllWithoutBuild(); + helper.command.export(); + mainBeforeDiverge = helper.scopeHelper.cloneLocalScope(); + + helper.command.createLane(); + helper.command.snapAllComponentsWithoutBuild('--unmodified'); + helper.command.export(); + + helper.scopeHelper.getClonedLocalScope(mainBeforeDiverge); + helper.npm.addFakeNpmPackage('ramda', '0.0.21'); + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { ramda: '0.0.21' } }); + helper.command.snapAllComponentsWithoutBuild(); + helper.command.export(); + + helper.scopeHelper.reInitLocalScope(); + helper.scopeHelper.addRemoteScope(); + helper.command.importLane('dev', '--skip-dependency-installation'); + helper.command.mergeLane('main', '--no-snap --skip-dependency-installation --ignore-config-changes'); + }); + it('should not delete the previously deps set', () => { + const deps = helper.command.getCompDepsIdsFromData('comp1'); + expect(deps).to.include('lodash'); + }); + }); }); diff --git a/scopes/workspace/workspace/aspects-merger.ts b/scopes/workspace/workspace/aspects-merger.ts index 889344a31ad2..a98407319738 100644 --- a/scopes/workspace/workspace/aspects-merger.ts +++ b/scopes/workspace/workspace/aspects-merger.ts @@ -97,6 +97,8 @@ export class AspectsMerger { const scopeExtensionsNonSpecific = new ExtensionDataList(...nonSpecific); const scopeExtensionsSpecific = new ExtensionDataList(...specific); + this.addConfigDepsFromModelToConfigMerge(scopeExtensionsSpecific, mergeConfigCombined); + const componentConfigFile = await this.workspace.componentConfigFile(componentId); if (componentConfigFile) { configFileExtensions = componentConfigFile.aspects.toLegacy(); @@ -243,6 +245,30 @@ export class AspectsMerger { } } + /** + * this is needed because if the mergeConfig has a policy, it will be used, and any other policy along the line will be ignored. + * in case the model has some dependencies that were set explicitly they're gonna be ignored. + * this makes sure to add them to the policy of the mergeConfig. + * in a way, this is similar to what we do when a user is running `bit deps set` and the component had previous dependencies set, + * we copy those dependencies along with the current one to the .bitmap file, so they won't get lost. + */ + private addConfigDepsFromModelToConfigMerge( + scopeExtensionsSpecific: ExtensionDataList, + mergeConfig?: Record + ) { + const mergeConfigPolicy = mergeConfig?.[DependencyResolverAspect.id]?.policy; + if (!mergeConfigPolicy) return; + const policy = scopeExtensionsSpecific.findCoreExtension(DependencyResolverAspect.id)?.config.policy; + if (!policy) return; + Object.keys(policy).forEach((key) => { + if (!mergeConfigPolicy[key]) { + mergeConfigPolicy[key] = policy[key]; + return; + } + mergeConfigPolicy[key] = merge(mergeConfigPolicy[key], policy[key]); + }); + } + private getUnmergedData(componentId: ComponentID): UnmergedComponent | undefined { return this.workspace.scope.legacyScope.objects.unmergedComponents.getEntry(componentId); } diff --git a/src/e2e-helper/e2e-command-helper.ts b/src/e2e-helper/e2e-command-helper.ts index 69118b529940..40836a421975 100644 --- a/src/e2e-helper/e2e-command-helper.ts +++ b/src/e2e-helper/e2e-command-helper.ts @@ -716,6 +716,7 @@ export default class CommandHelper { return showConfig.data.dependencies; } + /** returns the ids without the versions */ getCompDepsIdsFromData(compId: string): string[] { const aspectConf = this.showAspectConfig(compId, Extensions.dependencyResolver); return aspectConf.data.dependencies.map((dep) => dep.id); From 855856e0abe886cd59675a389116c055deaa1a2a Mon Sep 17 00:00:00 2001 From: David First Date: Tue, 12 Mar 2024 19:37:15 -0400 Subject: [PATCH 2/2] fix tests --- scopes/workspace/workspace/aspects-merger.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scopes/workspace/workspace/aspects-merger.ts b/scopes/workspace/workspace/aspects-merger.ts index a98407319738..d02f27df002e 100644 --- a/scopes/workspace/workspace/aspects-merger.ts +++ b/scopes/workspace/workspace/aspects-merger.ts @@ -258,14 +258,15 @@ export class AspectsMerger { ) { const mergeConfigPolicy = mergeConfig?.[DependencyResolverAspect.id]?.policy; if (!mergeConfigPolicy) return; - const policy = scopeExtensionsSpecific.findCoreExtension(DependencyResolverAspect.id)?.config.policy; - if (!policy) return; - Object.keys(policy).forEach((key) => { + const scopePolicy = scopeExtensionsSpecific.findCoreExtension(DependencyResolverAspect.id)?.config.policy; + if (!scopePolicy) return; + Object.keys(scopePolicy).forEach((key) => { if (!mergeConfigPolicy[key]) { - mergeConfigPolicy[key] = policy[key]; + mergeConfigPolicy[key] = scopePolicy[key]; return; } - mergeConfigPolicy[key] = merge(mergeConfigPolicy[key], policy[key]); + // mergeConfigPolicy should take precedence over scopePolicy + mergeConfigPolicy[key] = { ...scopePolicy[key], ...mergeConfigPolicy[key] }; }); }