fix(dep-resolver), validate deps-resolver aspect data (specifically "version" field) before saving it (#9648)

Currently, the "env.jsonc" can have any string in the "version" field,
and it'll be saved into the objects incorrectly.
This PR makes the following validations:
1. "version" field needs to be a semver/semver-range valid.
2. for components, it can be a hash, in case the version is a snap.
3. the `*` range is valid only for packages, not for components. The
reason is that for components, in case it was snapped on a lane, the
package on bit cloud doesn't have "latest" tag, as a result, the
installer fails.

Author: davidfirst

.eslintrc.js

@@ -107,7 +107,7 @@ module.exports = {
         ignoreTemplateLiterals: true,
       },
     ],
-    'max-lines': [2, 1800],
+    'max-lines': [2, 2000],
     'func-names': [0],
 
     // ERRORS OF plugin:react/recommended

components/legacy/e2e-helper/npm-ci-registry.ts

@@ -2,6 +2,7 @@
 import { addUser, REGISTRY_MOCK_PORT, start as startRegistryMock, prepare } from '@pnpm/registry-mock';
 import { ChildProcess } from 'child_process';
 import { fetch } from '@pnpm/fetch';
+import fs from 'fs-extra';
 import execa from 'execa';
 import * as path from 'path';
 
@@ -148,6 +149,22 @@ export class NpmCiRegistry {
     this.helper.command.runCmd('npm publish', extractedDir);
   }
 
+  /**
+   * publish an empty package to the registry.
+   * it's helpful in case a package is needed. not a component.
+   * instead of going to the NPM, this method is faster.
+   */
+  publishPackage(packageName: string, version = '0.0.1') {
+    const dir = this.helper.fs.createNewDirectory();
+    this.helper.command.runCmd(`npm init -y`, dir);
+    // change package.json according to the packageName and version
+    const packageJson = fs.readJsonSync(path.join(dir, 'package.json'));
+    packageJson.name = packageName;
+    packageJson.version = version;
+    fs.writeJsonSync(path.join(dir, 'package.json'), packageJson);
+    this.helper.command.runCmd('npm publish', dir);
+  }
+
   configureCiInPackageJsonHarmony() {
     const pkg = {
       packageJson: {

e2e/functionalities/peer-dependencies.e2e.3.ts

@@ -263,7 +263,7 @@ describe('peer-dependencies functionality', function () {
       helper = new Helper();
       helper.scopeHelper.reInitWorkspace();
       helper.fixtures.populateComponents(2);
-      helper.command.dependenciesSet('comp1', `@${helper.scopes.remote}/comp2@*`, '--peer');
+      helper.command.dependenciesSet('comp1', `@${helper.scopes.remote}/comp2@+`, '--peer');
       helper.command.snapAllComponents();
       helper.command.build();
       workspaceCapsulesRootDir = helper.command.capsuleListParsed().workspaceCapsulesRootDir;
@@ -273,28 +273,28 @@ describe('peer-dependencies functionality', function () {
       const peerDepData = output.peerDependencies[0];
       expect(peerDepData.id).to.startWith(`${helper.scopes.remote}/comp2`);
       expect(peerDepData.packageName).to.startWith(`@${helper.scopes.remote}/comp2`);
-      expect(peerDepData.versionRange).to.startWith('*');
+      expect(peerDepData.versionRange).to.startWith('+');
       const depResolver = output.extensions.find(({ name }) => name === 'teambit.dependencies/dependency-resolver');
       const peerDep = depResolver.data.dependencies[0];
       expect(peerDep.packageName).to.eq(`@${helper.scopes.remote}/comp2`);
       expect(peerDep.lifecycle).to.eq('peer');
-      expect(peerDep.versionRange).to.eq('*');
+      expect(peerDep.versionRange).to.eq('+');
     });
     it('should save the peer dependency in the scope data', () => {
       const comp = helper.command.catComponent(`comp1@latest`);
       const depResolver = comp.extensions.find(({ name }) => name === 'teambit.dependencies/dependency-resolver');
       const peerDep = depResolver.data.dependencies[0];
       expect(peerDep.packageName).to.eq(`@${helper.scopes.remote}/comp2`);
       expect(peerDep.lifecycle).to.eq('peer');
-      expect(peerDep.versionRange).to.eq('*');
+      expect(peerDep.versionRange).to.eq('+');
     });
     it('adds peer dependency to the generated package.json', () => {
       const { head } = helper.command.catComponent('comp1');
       const pkgJson = fs.readJsonSync(
         path.join(workspaceCapsulesRootDir, `${helper.scopes.remote}_comp1@${head}/package.json`)
       );
       expect(pkgJson.peerDependencies).to.deep.equal({
-        [`@${helper.scopes.remote}/comp2`]: '*',
+        [`@${helper.scopes.remote}/comp2`]: '+',
       });
     });
   });

e2e/harmony/dependency-resolver.e2e.ts

@@ -390,60 +390,65 @@ describe('dependency-resolver extension', function () {
   });
   (supportNpmCiRegistryTesting ? describe : describe.skip)('env.jsonc with policy.peer version="*"', () => {
     let npmCiRegistry: NpmCiRegistry;
+    const examplePkg = '@ci/lodash';
     before(async () => {
       helper = new Helper({ scopesOptions: { remoteScopeWithDot: true } });
       helper.scopeHelper.setWorkspaceWithRemoteScope();
       npmCiRegistry = new NpmCiRegistry(helper);
       await npmCiRegistry.init();
       npmCiRegistry.configureCiInPackageJsonHarmony();
+      npmCiRegistry.publishPackage(examplePkg, '0.0.1');
 
-      helper.fixtures.populateComponents(1);
-      helper.command.tagAllComponents();
       helper.env.setEmptyEnv();
       helper.fs.outputFile('empty-env/env.jsonc', `{
   "policy": {
     "peers": [
       {
-        "name": "${helper.general.getPackageNameByCompName('comp1')}",
+        "name": "${examplePkg}",
         "version": "*",
         "supportedRange": "*"
       }
     ]
   }
 }
 `);
-      helper.command.tagAllComponents(); // it'll tag only empty-env.
+      helper.command.tagAllComponents();
       helper.command.export();
     });
     after(() => {
       npmCiRegistry.destroy();
     });
-    function validateDepData(expectedVersion: string) {
+    function validatePkgData() {
       const comp = helper.command.catComponent(`${helper.scopes.remote}/empty-env@latest`);
       const depResolverExt = comp.extensions.find((e) => e.name === Extensions.dependencyResolver);
-      const policy = depResolverExt.data.policy.find(p => p.dependencyId === helper.general.getPackageNameByCompName('comp1'));
+      const policy = depResolverExt.data.policy.find(p => p.dependencyId === examplePkg);
       expect(policy.value.version).to.equal('*');
-      const data =  depResolverExt.data.dependencies.find(p => p.packageName === helper.general.getPackageNameByCompName('comp1'));
-      expect(data.version).to.equal(expectedVersion);
-      expect(data.componentId.version).to.equal(expectedVersion);
+      const data =  depResolverExt.data.dependencies.find(p => p.id === examplePkg);
+      expect(data.version).to.equal('*');
     }
     it('should not break and save the policy correctly with the *', () => {
-      validateDepData('0.0.1');
+      validatePkgData();
     });
-    describe('making a new version of the env dep', () => {
+    describe('publishing a new version of the package and re-tagging', () => {
       before(() => {
+        npmCiRegistry.publishPackage(examplePkg, '0.0.2');
         helper.command.tagAllComponents('--unmodified');
         helper.command.export();
       });
       it('should update the dep in the env model', () => {
-        validateDepData('0.0.2');
+        validatePkgData();
       });
-      it('should be able to install the env on a new workspace with no errors', () => {
+      it('should be able to install the env on a new workspace with no errors and install the latest of the pkg dep', () => {
         helper.scopeHelper.reInitWorkspace();
         helper.scopeHelper.addRemoteScope();
         helper.command.install(helper.general.getPackageNameByCompName('empty-env'));
-        const pkgJson = helper.fs.readJsonFile(`node_modules/${helper.general.getPackageNameByCompName('empty-env')}/package.json`);
-        expect(pkgJson.dependencies[`${helper.general.getPackageNameByCompName('comp1')}`]).to.equal('0.0.2');
+
+        const envPkgJson = helper.fs.readJsonFile(`node_modules/${helper.general.getPackageNameByCompName('empty-env')}/package.json`);
+        expect(envPkgJson.dependencies[examplePkg]).to.equal('*');
+
+        const pkgJsonPath = path.join('node_modules', '.pnpm/@ci+lodash@0.0.2/node_modules/@ci/lodash/package.json');
+        const pkgJson = helper.fs.readJsonFile(pkgJsonPath);
+        expect(pkgJson.version).to.equal('0.0.2');
       });
     });
   });

scopes/dependencies/dependency-resolver/dependency-resolver.main.runtime.ts

@@ -1,4 +1,5 @@
 import multimatch from 'multimatch';
+import { isSnap } from '@teambit/component-version';
 import mapSeries from 'p-map-series';
 import { DEPS_GRAPH, isFeatureEnabled } from '@teambit/harmony.modules.feature-toggle';
 import { MainRuntime } from '@teambit/cli';
@@ -101,7 +102,7 @@ export { ProxyConfig, NetworkConfig } from '@teambit/scope.network';
 export interface DependencyResolverComponentData {
   packageName: string;
   policy: SerializedVariantPolicy;
-  dependencies: SerializedDependency;
+  dependencies: SerializedDependency[];
 }
 
 export interface DependencyResolverVariantConfig {
@@ -1376,6 +1377,40 @@ export class DependencyResolverMain {
     return manifest;
   }
 
+  validateAspectData(data: DependencyResolverComponentData) {
+    const errorPrefix = `failed validating ${DependencyResolverAspect.id} aspect-data.`;
+    let errorMsg: undefined | string;
+    data.dependencies?.forEach((dep) => {
+      const isVersionValid = Boolean(semver.valid(dep.version) || semver.validRange(dep.version));
+      if (isVersionValid) return;
+      if (dep.__type === COMPONENT_DEP_TYPE && isSnap(dep.version)) return;
+      errorMsg = `${errorPrefix} the dependency version "${dep.version}" of ${dep.id} is not a valid semver version or range`;
+    });
+    data.policy?.forEach((policy) => {
+      const policyVersion = policy.value.version;
+      const allowedSpecialChars = ['+', '-'];
+      if (policyVersion === '*') {
+        // this is only valid for packages, not for components.
+        const isComp = data.dependencies.find(d => d.__type === COMPONENT_DEP_TYPE
+          && d.packageName === policy.dependencyId);
+        if (!isComp) return;
+        errorMsg = `${errorPrefix} the policy version "${policyVersion}" of ${policy.dependencyId} is not valid for components, only for packages.
+as an alternative, you can use "+" to keep the same version installed in the workspace`;
+      }
+      const isVersionValid = Boolean(
+        semver.valid(policyVersion) ||
+          semver.validRange(policyVersion) ||
+          allowedSpecialChars.includes(policyVersion)
+      );
+      if (isVersionValid) return;
+      errorMsg = `${errorPrefix} the policy version "${policyVersion}" of ${policy.dependencyId} is not a valid semver version or range`;
+    });
+
+    if (errorMsg) {
+      return { errorMsg, minBitVersion: '1.9.107' };
+    }
+  }
+
   /**
    * Return a list of outdated policy dependencies.
    */

scopes/harmony/aspect/aspect.main.runtime.ts

@@ -3,7 +3,7 @@ import { AspectLoaderAspect, AspectLoaderMain } from '@teambit/aspect-loader';
 import { LoggerAspect, LoggerMain } from '@teambit/logger';
 import { BuilderAspect, BuilderMain } from '@teambit/builder';
 import { compact, merge } from 'lodash';
-import { EnvPolicyConfigObject } from '@teambit/dependency-resolver';
+import { DependencyResolverAspect, DependencyResolverMain, EnvPolicyConfigObject } from '@teambit/dependency-resolver';
 import { BitError } from '@teambit/bit-error';
 import { CLIAspect, CLIMain, MainRuntime } from '@teambit/cli';
 import { EnvContext, Environment, EnvsAspect, EnvsMain, EnvTransformer } from '@teambit/envs';
@@ -39,7 +39,8 @@ export class AspectMain {
     readonly aspectEnv: AspectEnv,
     private envs: EnvsMain,
     private workspace: Workspace,
-    private aspectLoader: AspectLoaderMain
+    private aspectLoader: AspectLoaderMain,
+    private depsResolver: DependencyResolverMain
   ) {}
 
   /**
@@ -223,6 +224,11 @@ export class AspectMain {
       const result = this.envs.validateEnvId(envExt);
       if (result) return result;
     }
+    const depResolverExt = extensionDataList.findCoreExtension(DependencyResolverAspect.id);
+    if (depResolverExt) {
+      const result = this.depsResolver.validateAspectData(depResolverExt.data as any);
+      if (result) return result;
+    }
   }
 
   static runtime = MainRuntime;
@@ -238,10 +244,12 @@ export class AspectMain {
     LoggerAspect,
     WorkerAspect,
     DevFilesAspect,
+    DependencyResolverAspect,
   ];
 
   static async provider(
-    [react, envs, builder, aspectLoader, compiler, generator, workspace, cli, loggerMain, workerMain, devFilesMain]: [
+    [react, envs, builder, aspectLoader, compiler, generator, workspace, cli, loggerMain, workerMain, devFilesMain,
+      depResolver]: [
       ReactMain,
       EnvsMain,
       BuilderMain,
@@ -253,6 +261,7 @@ export class AspectMain {
       LoggerMain,
       WorkerMain,
       DevFilesMain,
+      DependencyResolverMain
     ],
     config,
     slots,
@@ -275,7 +284,7 @@ export class AspectMain {
       const envContext = new EnvContext(ComponentID.fromString(ReactAspect.id), loggerMain, workerMain, harmony);
       generator.registerComponentTemplate(() => getTemplates(envContext));
     }
-    const aspectMain = new AspectMain(aspectEnv as AspectEnv, envs, workspace, aspectLoader);
+    const aspectMain = new AspectMain(aspectEnv as AspectEnv, envs, workspace, aspectLoader, depResolver);
     const aspectCmd = new AspectCmd();
     aspectCmd.commands = [
       new ListAspectCmd(aspectMain),