From 63459aa36d5d6ae13562e793c465221bdf114b84 Mon Sep 17 00:00:00 2001 From: David First Date: Thu, 1 Aug 2019 16:48:55 -0400 Subject: [PATCH 1/2] implements #1865, allow overriding package.json props of a component via overrides key --- CHANGELOG.md | 1 + e2e/functionalities/workspace-config.e2e.3.js | 67 +++++++++++++++++++ .../component-ops/component-writer.js | 12 ++++ .../component-ops/components-object-diff.js | 1 + src/consumer/config/component-overrides.js | 15 ++--- src/consumer/config/consumer-overrides.js | 20 +++--- src/scope/version-validator.js | 19 +++--- 7 files changed, 109 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c914b91750b..b4cd74a28fa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +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` - [#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 diff --git a/e2e/functionalities/workspace-config.e2e.3.js b/e2e/functionalities/workspace-config.e2e.3.js index e9874867a7ab..a0ceae43ef8c 100644 --- a/e2e/functionalities/workspace-config.e2e.3.js +++ b/e2e/functionalities/workspace-config.e2e.3.js @@ -1297,6 +1297,73 @@ describe('workspace config', function () { expect(showBar.compiler).to.be.null; }); }); + describe('override package.json values', () => { + before(() => { + helper.setNewLocalAndRemoteScopes(); + helper.createComponentBarFoo(); + helper.addComponentBarFoo(); + const overrides = { + 'bar/*': { + bin: 'my-bin-file.js' + } + }; + helper.addOverridesToBitJson(overrides); + }); + it('bit show should show the overrides', () => { + const show = helper.showComponentParsed('bar/foo'); + expect(show.overrides) + .to.have.property('bin') + .equal('my-bin-file.js'); + }); + describe('tag, export and import the component', () => { + before(() => { + helper.tagAllComponents(); + helper.exportAllComponents(); + helper.reInitLocalScope(); + helper.addRemoteScope(); + helper.importComponent('bar/foo'); + }); + it('should write the values into package.json file', () => { + const packageJson = helper.readPackageJson(path.join(helper.localScopePath, 'components/bar/foo')); + expect(packageJson) + .to.have.property('bin') + .that.equals('my-bin-file.js'); + }); + it('should not show the component as modified', () => { + const status = helper.status(); + expect(status).to.have.string(statusWorkspaceIsCleanMsg); + }); + describe('changing the value in the package.json directly (not inside overrides)', () => { + before(() => { + const compDir = path.join(helper.localScopePath, 'components/bar/foo'); + const packageJson = helper.readPackageJson(compDir); + packageJson.bin = 'my-new-file.js'; + helper.writePackageJson(packageJson, compDir); + }); + it('should not show the component as modified', () => { + const status = helper.status(); + expect(status).to.not.have.string('modified components'); + }); + }); + describe('changing the value in the package.json inside overrides', () => { + before(() => { + const compDir = path.join(helper.localScopePath, 'components/bar/foo'); + const packageJson = helper.readPackageJson(compDir); + packageJson.bit.overrides.bin = 'my-new-file.js'; + helper.writePackageJson(packageJson, compDir); + }); + it('should show the component as modified', () => { + const status = helper.status(); + expect(status).to.have.string('modified components'); + }); + it('bit diff should show the field diff', () => { + const diff = helper.diff('bar/foo'); + expect(diff).to.have.string('my-bin-file.js'); + expect(diff).to.have.string('my-new-file.js'); + }); + }); + }); + }); }); describe('basic validations', () => { before(() => { diff --git a/src/consumer/component-ops/component-writer.js b/src/consumer/component-ops/component-writer.js index 13ad709733ed..33445edf98a4 100644 --- a/src/consumer/component-ops/component-writer.js +++ b/src/consumer/component-ops/component-writer.js @@ -177,6 +177,7 @@ export default class ComponentWriter { componentConfig.tester = this.component.tester ? this.component.tester.toBitJsonObject('.') : {}; packageJson.addOrUpdateProperty('bit', componentConfig.toPlainObject()); this._mergeChangedPackageJsonProps(packageJson); + this._mergePackageJsonPropsFromOverrides(packageJson); await this._populateEnvFilesIfNeeded(); this.component.dataToPersist.addFile(packageJson.toVinylFile()); if (distPackageJson) this.component.dataToPersist.addFile(distPackageJson.toVinylFile()); @@ -252,6 +253,17 @@ export default class ComponentWriter { } } + /** + * these changes were entered manually by a user via `overrides` key + */ + _mergePackageJsonPropsFromOverrides(packageJson: PackageJsonFile) { + const valuesToMerge = this.component.overrides.componentOverridesPackageJsonData; + packageJson.mergePackageJsonObject(valuesToMerge); + } + + /** + * these are changes done by a compiler + */ _mergeChangedPackageJsonProps(packageJson: PackageJsonFile) { if (!this.component.packageJsonChangedProps) return; const valuesToMerge = this._replaceDistPathTemplateWithCalculatedDistPath(packageJson); diff --git a/src/consumer/component-ops/components-object-diff.js b/src/consumer/component-ops/components-object-diff.js index 5b92979b8710..7e816a13c3f7 100644 --- a/src/consumer/component-ops/components-object-diff.js +++ b/src/consumer/component-ops/components-object-diff.js @@ -85,6 +85,7 @@ export function componentToPrintableForDiff(component: Component): Object { obj.overridesDependencies = parsePackages(overrides.dependencies); obj.overridesDevDependencies = parsePackages(overrides.devDependencies); obj.overridesPeerDependencies = parsePackages(overrides.peerDependencies); + obj.overridesPackageJsonProps = JSON.stringify(component.overrides.componentOverridesPackageJsonData); return obj; } diff --git a/src/consumer/config/component-overrides.js b/src/consumer/config/component-overrides.js index 8ab91caaf6b0..f72e0e31694b 100644 --- a/src/consumer/config/component-overrides.js +++ b/src/consumer/config/component-overrides.js @@ -8,7 +8,7 @@ import { OVERRIDE_COMPONENT_PREFIX } from '../../constants'; import type { ConsumerOverridesOfComponent } from './consumer-overrides'; -import { dependenciesFields } from './consumer-overrides'; +import { dependenciesFields, overridesSystemFields, nonPackageJsonFields } from './consumer-overrides'; export type ComponentOverridesData = { dependencies?: Object, @@ -59,14 +59,13 @@ export default class ComponentOverrides { // $FlowFixMe return new ComponentOverrides(R.clone(overridesFromModel), {}); } - - static componentOverridesDataFields() { - return dependenciesFields; - } get componentOverridesData() { - const fields = ComponentOverrides.componentOverridesDataFields(); - const isDependencyField = (val, field) => fields.includes(field); - return R.pickBy(isDependencyField, this.overrides); + const isNotSystemField = (val, field) => !overridesSystemFields.includes(field); + return R.pickBy(isNotSystemField, this.overrides); + } + get componentOverridesPackageJsonData() { + const isPackageJsonField = (val, field) => !nonPackageJsonFields.includes(field); + return R.pickBy(isPackageJsonField, this.overrides); } getComponentDependenciesWithVersion(): Object { const allDeps = Object.assign( diff --git a/src/consumer/config/consumer-overrides.js b/src/consumer/config/consumer-overrides.js index e6534ffb645a..371ff01057bc 100644 --- a/src/consumer/config/consumer-overrides.js +++ b/src/consumer/config/consumer-overrides.js @@ -19,7 +19,9 @@ export type ConsumerOverridesOfComponent = { export type ConsumerOverridesConfig = { [string]: ConsumerOverridesOfComponent }; export const dependenciesFields = ['dependencies', 'devDependencies', 'peerDependencies']; -const consumerOverridesPermittedFields = [...dependenciesFields, 'env']; +export const overridesForbiddenFields = ['name', 'main', 'version']; +export const overridesSystemFields = ['propagate']; +export const nonPackageJsonFields = [...dependenciesFields, ...overridesSystemFields]; export default class ConsumerOverrides { overrides: ConsumerOverridesConfig; @@ -47,10 +49,9 @@ export default class ConsumerOverrides { if (!current.propagate) { stopPropagation = true; } - consumerOverridesPermittedFields.forEach((field) => { - if (!current[field]) return; - if (!acc[field]) acc[field] = {}; + 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; @@ -59,8 +60,9 @@ export default class ConsumerOverrides { } else if (dependenciesFields.includes(field)) { // $FlowFixMe acc[field] = Object.assign(current[field], acc[field]); - } else { - throw new Error(`consumer-overrides, ${field} does not have a merge strategy`); + } else if (!overridesSystemFields.includes(field)) { + // $FlowFixMe propagate is a system field + acc[field] = current[field]; } }); return acc; @@ -154,9 +156,9 @@ export default class ConsumerOverrides { function validateComponentOverride(id, override) { validateUserInputType(message, override, `overrides.${id}`, 'object'); Object.keys(override).forEach((field) => { - if (!consumerOverridesPermittedFields.includes(field)) { - throw new GeneralError(`${message} found an unrecognized field "${field}" inside "overrides.${id}" property. -only the following fields are allowed: ${consumerOverridesPermittedFields.join(', ')}.`); + if (overridesForbiddenFields.includes(field)) { + throw new GeneralError(`${message} found a forbidden field "${field}" inside "overrides.${id}" property. +the following fields are not allowed: ${overridesForbiddenFields.join(', ')}.`); } if (dependenciesFields.includes(field)) { validateDependencyField(field, override, id); diff --git a/src/scope/version-validator.js b/src/scope/version-validator.js index baa52e5a30a2..6facaa1e0206 100644 --- a/src/scope/version-validator.js +++ b/src/scope/version-validator.js @@ -7,8 +7,8 @@ import VersionInvalid from './exceptions/version-invalid'; import { isValidPath } from '../utils'; import type Version from './models/version'; import { Dependencies } from '../consumer/component/dependencies'; -import ComponentOverrides from '../consumer/config/component-overrides'; import PackageJsonFile from '../consumer/component/package-json-file'; +import { overridesForbiddenFields, dependenciesFields } from '../consumer/config/consumer-overrides'; /** * make sure a Version instance is correct. throw an exceptions if it is not. @@ -174,18 +174,19 @@ export default function validateVersionInstance(version: Version): void { if (version.bindingPrefix) { validateType(message, version.bindingPrefix, 'bindingPrefix', 'string'); } - const overridesAllowedKeys = ComponentOverrides.componentOverridesDataFields(); const validateOverrides = (dependencies: Object, fieldName) => { const field = `overrides.${fieldName}`; - validateType(message, dependencies, field, 'object'); - Object.keys(dependencies).forEach((key) => { - validateType(message, key, `property name of ${field}`, 'string'); - validateType(message, dependencies[key], `version of "${field}.${key}"`, 'string'); - }); + if (dependenciesFields.includes(fieldName)) { + validateType(message, dependencies, field, 'object'); + Object.keys(dependencies).forEach((key) => { + validateType(message, key, `property name of ${field}`, 'string'); + validateType(message, dependencies[key], `version of "${field}.${key}"`, 'string'); + }); + } }; Object.keys(version.overrides).forEach((field) => { - if (!overridesAllowedKeys.includes(field)) { - throw new VersionInvalid(`${message}, the "overrides" has unidentified key "${field}"`); + if (overridesForbiddenFields.includes(field)) { + throw new VersionInvalid(`${message}, the "overrides" has a forbidden key "${field}"`); } // $FlowFixMe validateOverrides(version.overrides[field], field); From 2bde3eb7f38b8c82ed31f5fdaa42bf80baac5d1c Mon Sep 17 00:00:00 2001 From: David First Date: Thu, 1 Aug 2019 22:04:50 -0400 Subject: [PATCH 2/2] fix e2e-tests --- src/consumer/config/consumer-overrides.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/consumer/config/consumer-overrides.js b/src/consumer/config/consumer-overrides.js index 371ff01057bc..667882806c3c 100644 --- a/src/consumer/config/consumer-overrides.js +++ b/src/consumer/config/consumer-overrides.js @@ -20,7 +20,7 @@ export type ConsumerOverridesConfig = { [string]: ConsumerOverridesOfComponent } export const dependenciesFields = ['dependencies', 'devDependencies', 'peerDependencies']; export const overridesForbiddenFields = ['name', 'main', 'version']; -export const overridesSystemFields = ['propagate']; +export const overridesSystemFields = ['propagate', 'env']; export const nonPackageJsonFields = [...dependenciesFields, ...overridesSystemFields]; export default class ConsumerOverrides {