fix(deps): keep env.jsonc component peers after lane merge, support +^/+~ versions

components/legacy/constants/constants.ts

@@ -515,6 +515,12 @@ export const MANUALLY_REMOVE_ENVIRONMENT = '-';
 
 export const MANUALLY_ADD_DEPENDENCY = '+';
 
+/**
+ * placeholders for the version a dependency resolves to, optionally with a range prefix. e.g. "+^" => "^1.2.3".
+ * see resolvePlusVersion() of the dependency-resolver aspect.
+ */
+export const PLUS_VERSIONS = [MANUALLY_ADD_DEPENDENCY, '+^', '+~'];
+
 export const OVERRIDE_COMPONENT_PREFIX = '@bit/';
 
 export const ACCEPTABLE_NPM_VERSIONS = '>=5.0.0';

components/legacy/consumer-config/component-overrides.ts

@@ -1,7 +1,7 @@
 import { pickBy, keys, isEmpty, cloneDeep, get, merge } from 'lodash';
 import type { ComponentID } from '@teambit/component-id';
 import {
-  MANUALLY_ADD_DEPENDENCY,
+  PLUS_VERSIONS,
   MANUALLY_REMOVE_DEPENDENCY,
   OVERRIDE_COMPONENT_PREFIX,
   DEPENDENCIES_FIELDS,
@@ -143,7 +143,7 @@ export class ComponentOverrides {
     }, {});
   }
   _isValidVersion(ver: string) {
-    return ver !== MANUALLY_ADD_DEPENDENCY && ver !== MANUALLY_REMOVE_DEPENDENCY;
+    return !PLUS_VERSIONS.includes(ver) && ver !== MANUALLY_REMOVE_DEPENDENCY;
   }
   getIgnored(field: string): string[] {
     const filtered = pickBy(this.overrides[field] || {}, (dep) => {

components/legacy/e2e-helper/e2e-snapping-helper.ts

@@ -23,6 +23,8 @@ export default class SnappingHelper {
       updateDependents?: boolean;
       push?: boolean;
       message?: string;
+      tag?: boolean;
+      skipLoadingAspects?: boolean;
     } = {}
   ): Promise<void> {
     const runnerPath = path.resolve(__dirname, 'snap-from-scope-runner.js');

components/legacy/e2e-helper/snap-from-scope-runner.ts

@@ -11,6 +11,7 @@
 import { loadBit } from '@teambit/bit';
 import type { SnappingMain } from '@teambit/snapping';
 import { SnappingAspect } from '@teambit/snapping';
+import { ComponentIdList } from '@teambit/component-id';
 
 async function main(): Promise<void> {
   const [, , scopePath, snapDataJson, optionsJson] = process.argv;
@@ -24,6 +25,9 @@ async function main(): Promise<void> {
     updateDependents: options.updateDependents,
     push: options.push,
     message: options.message,
+    tag: options.tag,
+    // skips loading the components' aspects (e.g. envs) in capsules, which requires their packages in a registry
+    loadAspectOnlyForIds: options.skipLoadingAspects ? new ComponentIdList() : undefined,
     build: false,
     disableTagAndSnapPipelines: true,
   });

e2e/harmony/lanes/lane-env-jsonc-dep.e2e.ts

@@ -1,4 +1,7 @@
 import { expect } from 'chai';
+import fs from 'fs-extra';
+import path from 'path';
+import { Extensions } from '@teambit/legacy.constants';
 import { Helper } from '@teambit/legacy.e2e-helper';
 
 /**
@@ -129,4 +132,94 @@ export default new MyEnv();
       });
     });
   });
+
+  /**
+   * an env.jsonc peer that is a component and is part of the lane, set with a "+" placeholder.
+   * on the lane, it should follow the lane snap. once the lane is merged and released (tagged from a bare scope,
+   * the way bit.cloud releases a change-request), it should follow the new tag, with the "^" prefix.
+   * it also covers a bug where releasing from a bare scope dropped the component peer-dependencies from the
+   * dependency-resolver data, so on main they appeared only in the policy and every new lane showed them as new.
+   */
+  describe('env.jsonc peer with "+^" that is a lane component, merged and released from a bare scope', () => {
+    let comp2PkgName: string;
+    let comp2LaneSnap: string;
+    let comp1DepsOnLane: Record<string, any>[];
+    const getComp2DepData = (deps: Record<string, any>[]) => deps.find((d) => d.packageName === comp2PkgName);
+    before(async () => {
+      helper.scopeHelper.setWorkspaceWithRemoteScope();
+      helper.fixtures.populateComponents(2); // comp1 requires comp2
+      helper.env.setEmptyEnv();
+      helper.command.setEnv('comp1', 'empty-env');
+      helper.command.tagAllWithoutBuild();
+      comp2PkgName = helper.general.getPackageNameByCompName('comp2', false);
+      helper.fixtures.generateEnvJsoncFile('empty-env', {
+        policy: { peers: [{ name: comp2PkgName, version: '+', supportedRange: '+^' }] },
+      });
+      helper.command.tagAllWithoutBuild();
+      helper.command.export();
+
+      helper.command.createLane('dev');
+      helper.fs.appendFile('comp2/index.js', '\n// v2');
+      helper.command.snapAllComponentsWithoutBuild();
+      helper.command.export();
+      comp2LaneSnap = helper.command.getHeadOfLane('dev', 'comp2');
+      comp1DepsOnLane = helper.command.showDependenciesData('comp1');
+
+      // merge the lane into main, then release it the way bit.cloud does: tag from a bare scope.
+      helper.command.switchLocalLane('main', '-x');
+      helper.command.mergeLane('dev', '-x');
+      helper.command.export();
+      const bareScope = helper.scopeHelper.getNewBareScope('-bare-release');
+      helper.scopeHelper.addRemoteScope(helper.scopes.remotePath, bareScope.scopePath);
+      await helper.snapping.snapFromScope(
+        bareScope.scopePath,
+        ['comp1', 'comp2', 'empty-env'].map((name) => ({
+          componentId: `${helper.scopes.remote}/${name}`,
+          message: 'release',
+        })),
+        { tag: true, push: true, skipLoadingAspects: true }
+      );
+    });
+    it('on the lane, the peer should be the lane snap and keep the "+^" as the range', () => {
+      expect(getComp2DepData(comp1DepsOnLane)).to.include({
+        lifecycle: 'peer',
+        version: comp2LaneSnap,
+        versionRange: '+^',
+      });
+    });
+    describe('after the release', () => {
+      // comp2 was tagged once (0.0.1) before the lane, so the release tags it as 0.0.2
+      const comp2ReleasedVersion = '0.0.2';
+      it('should keep the component peer in the dependency-resolver data, pointing to the new tag', () => {
+        const depsResolverData = helper.command.getAspectsDataFromId(
+          `${helper.scopes.remote}/comp1`,
+          Extensions.dependencyResolver,
+          helper.scopes.remotePath
+        );
+        expect(getComp2DepData(depsResolverData.dependencies), 'the peer was dropped').to.include({
+          __type: 'component',
+          lifecycle: 'peer',
+          version: comp2ReleasedVersion,
+          versionRange: '+^',
+        });
+      });
+      describe('checking out the released versions in the workspace', () => {
+        before(() => {
+          helper.command.import();
+          helper.command.checkoutHead('--all', '-x');
+        });
+        it('should not show any component as modified', () => {
+          helper.command.expectStatusToBeClean();
+        });
+        it('should write the peer with "^" and the new tag into the package.json of the capsule', () => {
+          helper.command.build('comp1');
+          const comp1Capsule = helper.command
+            .capsuleListParsed()
+            .capsules.find((capsule: string) => capsule.includes('comp1')) as string;
+          const pkgJson = fs.readJsonSync(path.join(comp1Capsule, 'package.json'));
+          expect(pkgJson.peerDependencies[comp2PkgName]).to.equal(`^${comp2ReleasedVersion}`);
+        });
+      });
+    });
+  });
 });

scopes/component/isolator/isolator.main.runtime.ts

@@ -28,7 +28,7 @@ import type {
   PackageManagerInstallOptions,
   NodeLinker,
 } from '@teambit/dependency-resolver';
-import { DependencyResolverAspect, KEY_NAME_BY_LIFECYCLE_TYPE } from '@teambit/dependency-resolver';
+import { DependencyResolverAspect, KEY_NAME_BY_LIFECYCLE_TYPE, resolvePlusVersion } from '@teambit/dependency-resolver';
 import type { Logger, LoggerMain, LongProcessLogger } from '@teambit/logger';
 import { LoggerAspect } from '@teambit/logger';
 import type { ComponentID } from '@teambit/component-id';
@@ -1501,8 +1501,9 @@ export class IsolatorMain {
         const keyName = KEY_NAME_BY_LIFECYCLE_TYPE[dep.lifecycle];
         const entry = dep.toManifest();
         if (entry) {
-          manifest[keyName][entry.packageName] =
-            dep.versionRange && dep.versionRange !== '+' ? dep.versionRange : version;
+          manifest[keyName][entry.packageName] = dep.versionRange
+            ? resolvePlusVersion(dep.versionRange, version)
+            : version;
         }
       });
       await Promise.all(promises);
@@ -1540,7 +1541,10 @@ export class IsolatorMain {
     clonedFiles.forEach((file) => file.updatePaths({ newBase: writeToPath }));
     dataToPersist.removePath(new RemovePath(writeToPath));
     clonedFiles.map((file) => dataToPersist.addFile(file));
-    const packageJson = this.preparePackageJsonToWrite(component, writeToPath, ids);
+    const shouldMergePkgJsonFromLastBuild = Boolean(
+      populateArtifactsFromComps && !opts?.populateArtifactsIgnorePkgJson
+    );
+    const packageJson = this.preparePackageJsonToWrite(component, writeToPath, ids, shouldMergePkgJsonFromLastBuild);
     if (!legacyComp.id.hasVersion()) {
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       packageJson.addOrUpdateProperty('version', semver.inc(legacyComp.version!, 'prerelease') || '0.0.1-0');
@@ -1552,7 +1556,7 @@ export class IsolatorMain {
     await PackageJsonTransformer.applyTransformers(component, packageJson as any);
     const valuesToMerge = legacyComp.overrides.componentOverridesPackageJsonData;
     packageJson.mergePackageJsonObject(valuesToMerge);
-    if (populateArtifactsFromComps && !opts?.populateArtifactsIgnorePkgJson) {
+    if (populateArtifactsFromComps && shouldMergePkgJsonFromLastBuild) {
       const compParent = this.getCompForArtifacts(component, populateArtifactsFromComps);
       this.mergePkgJsonFromLastBuild(compParent, packageJson);
     }
@@ -1605,10 +1609,16 @@ export class IsolatorMain {
     return compParent;
   }
 
+  /**
+   * @param includePeers whether to write the component peers. needed when the package.json of the last build is merged
+   * into this one (see mergePkgJsonFromLastBuild), which keeps only the dependencies that exist here. otherwise, a peer
+   * keeps the version of the last build, e.g. a lane snap of a component that was tagged since then.
+   */
   private preparePackageJsonToWrite(
     component: Component,
     bitDir: string,
-    componentDepsToIgnore: ComponentIdList
+    componentDepsToIgnore: ComponentIdList,
+    includePeers = false
   ): PackageJsonFile {
     const legacyComp: ConsumerComponent = component.state._consumer;
     const compDeps = this.dependencyResolver.getComponentDependencies(component);
@@ -1627,13 +1637,16 @@ export class IsolatorMain {
           id: depId,
           isDependency: true,
         });
-        acc[packageName] = versionWithRange || packageDependency;
+        acc[packageName] = versionWithRange
+          ? resolvePlusVersion(versionWithRange, packageDependency, versionWithRange)
+          : packageDependency;
         return acc;
       }, {});
     };
     const bitDependencies = getBitDependencies(legacyComp.dependencies.getAllIds());
     const bitDevDependencies = getBitDependencies(legacyComp.devDependencies.getAllIds());
     const bitExtensionDependencies = getBitDependencies(legacyComp.extensions.extensionsBitIds);
+    const bitPeerDependencies = includePeers ? getBitDependencies(legacyComp.peerDependencies.getAllIds()) : {};
     const packageJson = PackageJsonFile.createFromComponent(bitDir, legacyComp);
     const main = pathNormalizeToLinux(legacyComp.mainFile);
     packageJson.addOrUpdateProperty('main', main);
@@ -1643,6 +1656,7 @@ export class IsolatorMain {
         ...bitDevDependencies,
         ...bitExtensionDependencies,
       });
+      packageJsonFile.addPeerDependencies(bitPeerDependencies);
     };
     addDependencies(packageJson);
     const currentVersion = getComponentPackageVersion(component);

scopes/component/isolator/prepare-package-json-to-write.spec.ts

@@ -0,0 +1,43 @@
+import { expect } from 'chai';
+import { ComponentID, ComponentIdList } from '@teambit/component-id';
+import { ExtensionDataList } from '@teambit/legacy.extension-data';
+import { IsolatorMain } from './isolator.main.runtime';
+
+const depId = ComponentID.fromString('my-scope/dep@0.0.2');
+
+function preparePackageJson(componentDepsToIgnore: ComponentIdList, includePeers: boolean) {
+  const isolator = Object.create(IsolatorMain.prototype);
+  isolator.logger = { debug: () => {} };
+  isolator.dependencyResolver = { getComponentDependencies: () => [{ componentId: depId, versionRange: '+^' }] };
+  const noDeps = { getAllIds: () => new ComponentIdList() };
+  const legacyComp = {
+    id: ComponentID.fromString('my-scope/user@0.0.2'),
+    version: '0.0.2',
+    mainFile: 'index.js',
+    _getHomepage: () => undefined,
+    packageDependencies: {},
+    devPackageDependencies: {},
+    peerPackageDependencies: {},
+    extensions: new ExtensionDataList(),
+    dependencies: noDeps,
+    devDependencies: noDeps,
+    peerDependencies: { getAllIds: () => ComponentIdList.fromArray([depId]) },
+  };
+  const component = { id: legacyComp.id, state: { _consumer: legacyComp } };
+  return isolator.preparePackageJsonToWrite(component, '.', componentDepsToIgnore, includePeers).packageJsonObject;
+}
+
+describe('IsolatorMain.preparePackageJsonToWrite()', () => {
+  it('should write the component peers with their resolved version when asked to', () => {
+    const packageJson = preparePackageJson(new ComponentIdList(), true);
+    expect(Object.values(packageJson.peerDependencies)).to.deep.equal(['^0.0.2']);
+  });
+  it('should not write the component peers by default', () => {
+    const packageJson = preparePackageJson(new ComponentIdList(), false);
+    expect(packageJson.peerDependencies).to.deep.equal({});
+  });
+  it('should not write a component peer that is part of the capsules', () => {
+    const packageJson = preparePackageJson(ComponentIdList.fromArray([depId]), true);
+    expect(packageJson.peerDependencies).to.deep.equal({});
+  });
+});

scopes/component/snapping/snapping.main.runtime.ts

@@ -1408,7 +1408,11 @@ another option, in case this dependency is not in main yet is to remove all refe
     const updatedIds = ComponentIdList.fromArray([...currentBitIds, ...dependencies]);
     const componentIdStr = component.id.toString();
     const legacyComponent: ConsumerComponent = component.state._consumer;
-    const deps = [...legacyComponent.dependencies.get(), ...legacyComponent.devDependencies.get()];
+    const deps = [
+      ...legacyComponent.dependencies.get(),
+      ...legacyComponent.devDependencies.get(),
+      ...legacyComponent.peerDependencies.get(),
+    ];
     const dependenciesList = this.dependencyResolver.getDependencies(component);
     deps.forEach((dep) => {
       const updatedBitId = updatedIds.searchWithoutVersion(dep.id);

scopes/dependencies/dependencies/dependencies-loader/apply-overrides.spec.ts

@@ -0,0 +1,26 @@
+import { expect } from 'chai';
+import { ApplyOverrides } from './apply-overrides';
+
+function createApplyOverrides({ installed, inWorkspacePolicy }: { installed?: string; inWorkspacePolicy?: string }) {
+  const applyOverrides = Object.create(ApplyOverrides.prototype);
+  applyOverrides.depsResolver = {
+    getWorkspacePolicyManifest: () => ({ dependencies: inWorkspacePolicy ? { 'my-pkg': inWorkspacePolicy } : {} }),
+  };
+  applyOverrides._resolvePackageData = () => (installed ? { concreteVersion: installed } : undefined);
+  return applyOverrides;
+}
+
+describe('ApplyOverrides.resolveEnvPeerDepVersion()', () => {
+  it('should prefer the installed version by default', () => {
+    const applyOverrides = createApplyOverrides({ installed: '2.0.0', inWorkspacePolicy: '3.0.1' });
+    expect(applyOverrides.resolveEnvPeerDepVersion('my-pkg')).to.equal('2.0.0');
+  });
+  it('should prefer the workspace policy when asked to, as the installed version could be outdated', () => {
+    const applyOverrides = createApplyOverrides({ installed: '2.0.0', inWorkspacePolicy: '3.0.1' });
+    expect(applyOverrides.resolveEnvPeerDepVersion('my-pkg', true)).to.equal('3.0.1');
+  });
+  it('should fall back to the installed version when the workspace policy does not have the package', () => {
+    const applyOverrides = createApplyOverrides({ installed: '2.0.0' });
+    expect(applyOverrides.resolveEnvPeerDepVersion('my-pkg', true)).to.equal('2.0.0');
+  });
+});

scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts

@@ -14,6 +14,7 @@ import type { ResolvedPackageData } from '../resolve-pkg-data';
 import { resolvePackageData } from '../resolve-pkg-data';
 import type { Workspace } from '@teambit/workspace';
 import type { DependencyResolverMain } from '@teambit/dependency-resolver';
+import { isPlusVersion, resolvePlusVersion } from '@teambit/dependency-resolver';
 import type { Consumer } from '@teambit/legacy.consumer';
 import type { ComponentMap } from '@teambit/legacy.bit-map';
 import type { Logger } from '@teambit/logger';
@@ -716,7 +717,11 @@ export class ApplyOverrides {
       comp.versionRange = pkgVal;
       this.allDependencies.peerDependencies.push(comp);
     } else {
-      this.allPackagesDependencies[key][pkgName] = pkgVal;
+      // a package has no separate version and range, so a "+" placeholder is resolved here.
+      // a component dep keeps it in the versionRange, and it's resolved when generating the package.json
+      this.allPackagesDependencies[key][pkgName] = isPlusVersion(pkgVal)
+        ? resolvePlusVersion(pkgVal, this.resolveEnvPeerDepVersion(pkgName, true))
+        : pkgVal;
     }
 
     if (existsInCompsPeerDeps) {
@@ -750,11 +755,11 @@ export class ApplyOverrides {
     });
     const resolvedEnvPolicyManifest = Object.keys(envPolicyManifest).reduce((acc, pkgName) => {
       const version = envPolicyManifest[pkgName];
-      if (version !== '+') {
+      if (!isPlusVersion(version)) {
         acc[pkgName] = version;
         return acc;
       }
-      acc[pkgName] = this.resolveEnvPeerDepVersion(pkgName);
+      acc[pkgName] = resolvePlusVersion(version, this.resolveEnvPeerDepVersion(pkgName));
       return acc;
     }, {});
     // TODO: handle component deps once we support peers between components
@@ -765,21 +770,27 @@ export class ApplyOverrides {
   }
 
   /**
-   * in the env.jsonc file, a policy-peer package can have `+` sign in the version. it means that it should be resolved
+   * in the env.jsonc file, a policy-peer package can have `+` sign (or `+^`/`+~`) in the version. it means that it should be resolved
    * from the workspace. whatever version is installed/imported in the workspace, it should be used here.
    * in some cases, the package is not installed in the workspace, for example, the env is now imported without the
    * dep. so the dep is not in the node_modules.
    * strategy should be: .bitmap, workspace.jsonc, then model.
    * it's not in .bitmap, otherwise, it was linked and `_resolvePackageData` would have found it.
    * so either, it's in the workspace.jsonc or in the model.
+   * @param preferWorkspacePolicy check the workspace.jsonc before what's installed. needed for a package that the
+   * component gets with a plus version. what's installed for the component was installed according to the version it
+   * had before, so it doesn't follow an upgrade in the workspace.jsonc (e.g. by "bit update").
    */
-  private resolveEnvPeerDepVersion(pkgName: string): string {
+  private resolveEnvPeerDepVersion(pkgName: string, preferWorkspacePolicy = false): string {
+    const wsPolicy = this.depsResolver.getWorkspacePolicyManifest();
+    const wsVersion = wsPolicy?.dependencies?.[pkgName] || wsPolicy?.peerDependencies?.[pkgName];
+    if (preferWorkspacePolicy && wsVersion) {
+      return wsVersion;
+    }
     const resolved = this._resolvePackageData(pkgName);
     if (resolved && resolved.concreteVersion) {
       return resolved.concreteVersion;
     }
-    const wsPolicy = this.depsResolver.getWorkspacePolicyManifest();
-    const wsVersion = wsPolicy?.dependencies?.[pkgName] || wsPolicy?.peerDependencies?.[pkgName];
     if (wsVersion) {
       return wsVersion;
     }