merge package.json props from a more general rule to a specific one

CHANGELOG.md

@@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [unreleased]
 
-- [#1865](/~https://github.com/teambit/bit/issues/1865) allow override `package.json` props via `overrides`
+- [#1865](/~https://github.com/teambit/bit/issues/1865) allow adding `package.json` props via `overrides`
 - [#1837](/~https://github.com/teambit/bit/issues/1837) enable executing commands on remote components outside of bit-workspace
 - [#1774](/~https://github.com/teambit/bit/issues/1774) improve access errors and warn when sudo is used
 

e2e/functionalities/workspace-config.e2e.3.js

@@ -1316,9 +1316,11 @@ describe('workspace config', function () {
           .equal('my-bin-file.js');
       });
       describe('tag, export and import the component', () => {
+        let authorScope;
         before(() => {
           helper.tagAllComponents();
           helper.exportAllComponents();
+          authorScope = helper.cloneLocalScope();
           helper.reInitLocalScope();
           helper.addRemoteScope();
           helper.importComponent('bar/foo');
@@ -1361,16 +1363,96 @@ describe('workspace config', function () {
             expect(diff).to.have.string('my-bin-file.js');
             expect(diff).to.have.string('my-new-file.js');
           });
+          describe('tagging, exporting and re-importing as author', () => {
+            before(() => {
+              helper.tagAllComponents();
+              helper.exportAllComponents();
+              helper.getClonedLocalScope(authorScope);
+              helper.importComponent('bar/foo');
+            });
+            it('should not show the component as modified', () => {
+              const status = helper.status();
+              expect(status).to.not.have.string('modified components');
+            });
+            it('author bit.json should be rewritten to include a rule of the specific component', () => {
+              const bitJson = helper.readBitJson();
+              expect(bitJson.overrides)
+                .to.have.property(`${helper.remoteScope}/bar/foo`)
+                .that.deep.equals({ bin: 'my-new-file.js' });
+            });
+            it('bit show should display the modified field and not the original one', () => {
+              const show = helper.showComponentParsed('bar/foo');
+              expect(show.overrides)
+                .to.have.property('bin')
+                .that.equals('my-new-file.js');
+            });
+          });
         });
       });
     });
+    describe('propagating from a specific rule to a more general rule when propagate field is true', () => {
+      let show;
+      before(() => {
+        helper.setNewLocalAndRemoteScopes();
+        helper.createComponentBarFoo();
+        helper.addComponentBarFoo();
+        helper.addComponentBarFoo();
+        const overrides = {
+          '*': {
+            scripts: {
+              build: 'babel build'
+            }
+          },
+          'bar/*': {
+            bin: 'my-bin-file.js',
+            scripts: {
+              test: 'mocha test',
+              lint: 'eslint lint'
+            },
+            propagate: true
+          },
+          'bar/foo': {
+            scripts: {
+              test: 'jest test',
+              watch: 'babel watch'
+            },
+            propagate: true
+          }
+        };
+        helper.addOverridesToBitJson(overrides);
+        show = helper.showComponentParsed();
+      });
+      it('should not save the "propagate" field', () => {
+        expect(show.overrides).to.not.have.property('propagate');
+      });
+      it('should propagate to a more general rule and save string values that are not in the specific rule', () => {
+        expect(show.overrides)
+          .to.have.property('bin')
+          .that.equals('my-bin-file.js');
+      });
+      it('should propagate to a more general rule and merge objects that are in the specific rule', () => {
+        expect(show.overrides).to.have.property('scripts');
+        expect(show.overrides.scripts)
+          .to.have.property('build')
+          .that.equals('babel build');
+        expect(show.overrides.scripts)
+          .to.have.property('lint')
+          .that.equals('eslint lint');
+        expect(show.overrides.scripts)
+          .to.have.property('watch')
+          .that.equals('babel watch');
+      });
+      it('should let the more specific rule wins when it contradict a more general rule', () => {
+        expect(show.overrides.scripts).to.have.property('test');
+        expect(show.overrides.scripts.test).to.equals('jest test');
+        expect(show.overrides.scripts.test).not.to.equals('mocha test');
+      });
+    });
   });
   describe('basic validations', () => {
-    before(() => {
-      helper.reInitLocalScope();
-    });
     describe('when overrides is not an object', () => {
       before(() => {
+        helper.reInitLocalScope();
         const overrides = ['dependencies'];
         helper.addOverridesToBitJson(overrides);
       });
@@ -1381,6 +1463,7 @@ describe('workspace config', function () {
     });
     describe('when overrides of a component is not an object', () => {
       before(() => {
+        helper.reInitLocalScope();
         const overrides = {
           bar: 1234
         };
@@ -1393,9 +1476,10 @@ describe('workspace config', function () {
     });
     describe('when a forbidden field is added into overrides of a component', () => {
       before(() => {
+        helper.reInitLocalScope();
         const overrides = {
           bar: {
-            name: 'foo'
+            name: 'foo' // the name field of package.json is not permitted to change
           }
         };
         helper.addOverridesToBitJson(overrides);
@@ -1405,8 +1489,28 @@ describe('workspace config', function () {
         expect(output).to.have.string('found a forbidden field "name" inside "overrides.bar" property');
       });
     });
+    describe('when a non-compliant package.json field is added into overrides of a component', () => {
+      before(() => {
+        helper.reInitLocalScope();
+        helper.createComponentBarFoo();
+        helper.addComponentBarFoo();
+        const overrides = {
+          'bar/*': {
+            private: 'foo' // according to npm specs it should be boolean
+          }
+        };
+        helper.addOverridesToBitJson(overrides);
+      });
+      it('bit tag should throw an error', () => {
+        const output = helper.runWithTryCatch('bit tag -a');
+        expect(output).to.have.string(
+          'unable to save Version object, "overrides.private" is a package.json field but is not compliant with npm requirements. Type for field private, was expected to be boolean, not string'
+        );
+      });
+    });
     describe('when a dependency field is not an object', () => {
       before(() => {
+        helper.reInitLocalScope();
         const overrides = {
           bar: {
             dependencies: 1234
@@ -1421,6 +1525,7 @@ describe('workspace config', function () {
     });
     describe('when a dependency rule is not a string', () => {
       before(() => {
+        helper.reInitLocalScope();
         const overrides = {
           foo: {
             dependencies: {

src/consumer/config/consumer-overrides.js

@@ -19,7 +19,7 @@ export type ConsumerOverridesOfComponent = {
 export type ConsumerOverridesConfig = { [string]: ConsumerOverridesOfComponent };
 
 export const dependenciesFields = ['dependencies', 'devDependencies', 'peerDependencies'];
-export const overridesForbiddenFields = ['name', 'main', 'version'];
+export const overridesForbiddenFields = ['name', 'main', 'version', 'bit'];
 export const overridesSystemFields = ['propagate', 'env'];
 export const nonPackageJsonFields = [...dependenciesFields, ...overridesSystemFields];
 
@@ -33,42 +33,53 @@ export default class ConsumerOverrides {
     return new ConsumerOverrides(overrides);
   }
   getOverrideComponentData(bitId: BitId): ?ConsumerOverridesOfComponent {
-    const getMatches = (): string[] => {
-      const exactMatch = this.findExactMatch(bitId);
-      const matchByGlobPattern = Object.keys(this.overrides).filter(idStr => this.isMatchByWildcard(bitId, idStr));
-      const allMatches = matchByGlobPattern.sort(ConsumerOverrides.sortWildcards);
-      if (exactMatch) allMatches.unshift(exactMatch);
-      return allMatches;
-    };
-    const matches = getMatches();
-    if (!matches.length) return null;
+    const matches = this._getAllRulesMatchedById(bitId);
+    if (!matches.length) {
+      return null;
+    }
     const overrideValues = matches.map(match => this.overrides[match]);
     let stopPropagation = false;
     return overrideValues.reduce((acc, current) => {
       if (stopPropagation) return acc;
       if (!current.propagate) {
         stopPropagation = true;
       }
-      Object.keys(current).forEach((field) => {
-        if (field === 'env') {
-          if (!acc[field]) acc[field] = {};
-          ['compiler', 'tester'].forEach((envField) => {
-            // $FlowFixMe we made sure before that current.env is set
-            if (acc.env[envField] || !current.env[envField]) return;
-            acc.env[envField] = current.env[envField];
-          });
-        } else if (dependenciesFields.includes(field)) {
-          // $FlowFixMe
-          acc[field] = Object.assign(current[field], acc[field]);
-        } else if (!overridesSystemFields.includes(field)) {
-          // $FlowFixMe propagate is a system field
-          acc[field] = current[field];
-        }
-      });
+      this._updateSpecificOverridesWithGeneralOverrides(current, acc);
       return acc;
     }, {});
   }
-  isMatchByWildcard(bitId: BitId, idWithPossibleWildcard: string): boolean {
+  _updateSpecificOverridesWithGeneralOverrides(generalOverrides: Object, specificOverrides: Object) {
+    const isObjectAndNotArray = val => typeof val === 'object' && !Array.isArray(val);
+    Object.keys(generalOverrides).forEach((field) => {
+      switch (field) {
+        case 'env':
+          if (!specificOverrides[field]) specificOverrides[field] = {};
+          ['compiler', 'tester'].forEach((envField) => {
+            if (specificOverrides.env[envField] || !generalOverrides.env[envField]) return;
+            specificOverrides.env[envField] = generalOverrides.env[envField];
+          });
+          break;
+        case 'propagate':
+          // it's a system field, do nothing
+          break;
+        default:
+          if (isObjectAndNotArray(specificOverrides[field]) && isObjectAndNotArray(generalOverrides[field])) {
+            specificOverrides[field] = Object.assign(generalOverrides[field], specificOverrides[field]);
+          } else if (!specificOverrides[field]) {
+            specificOverrides[field] = generalOverrides[field];
+          }
+        // when specificOverrides[field] is set and not an object, do not override it by the general one
+      }
+    });
+  }
+  _getAllRulesMatchedById(bitId: BitId): string[] {
+    const exactMatch = this.findExactMatch(bitId);
+    const matchByGlobPattern = Object.keys(this.overrides).filter(idStr => this._isMatchByWildcard(bitId, idStr));
+    const allMatches = matchByGlobPattern.sort(ConsumerOverrides.sortWildcards);
+    if (exactMatch) allMatches.unshift(exactMatch);
+    return allMatches;
+  }
+  _isMatchByWildcard(bitId: BitId, idWithPossibleWildcard: string): boolean {
     if (!hasWildcard(idWithPossibleWildcard)) return false;
     return isBitIdMatchByWildcards(bitId, idWithPossibleWildcard);
   }