Skip to content

Commit

Permalink
Merge 2bde3eb into 5ffb681
Browse files Browse the repository at this point in the history
  • Loading branch information
davidfirst authored Aug 2, 2019
2 parents 5ffb681 + 2bde3eb commit 1cd3c06
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
67 changes: 67 additions & 0 deletions e2e/functionalities/workspace-config.e2e.3.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
12 changes: 12 additions & 0 deletions src/consumer/component-ops/component-writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/consumer/component-ops/components-object-diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
15 changes: 7 additions & 8 deletions src/consumer/config/component-overrides.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
20 changes: 11 additions & 9 deletions src/consumer/config/consumer-overrides.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', 'env'];
export const nonPackageJsonFields = [...dependenciesFields, ...overridesSystemFields];

export default class ConsumerOverrides {
overrides: ConsumerOverridesConfig;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 10 additions & 9 deletions src/scope/version-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 1cd3c06

Please sign in to comment.