Skip to content

Commit

Permalink
show parsing and resolving errors by "bit status" and prevent tagging…
Browse files Browse the repository at this point in the history
… it until fixed (#1109)
  • Loading branch information
davidfirst committed Jun 29, 2018
1 parent 887fbf1 commit 8280a79
Show file tree
Hide file tree
Showing 17 changed files with 276 additions and 217 deletions.
8 changes: 4 additions & 4 deletions .bitmap
Expand Up @@ -12,16 +12,16 @@
"mainFile": "src/plugins/wix/stylable.js",
"origin": "AUTHORED"
},
"david.bit-javascript/dependency-tree/types@0.0.1": {
"david.bit-javascript/dependency-tree/types@0.0.2": {
"files": [
{
"name": "dependency-tree-type.js",
"relativePath": "src/consumer/component/dependencies/dependency-resolver/types/dependency-tree-type.js",
"test": false,
"name": "dependency-tree-type.js"
"test": false
}
],
"mainFile": "src/consumer/component/dependencies/dependency-resolver/types/dependency-tree-type.js",
"origin": "AUTHORED"
},
"version": "13.0.0-dev.1"
"version": "13.0.1"
}
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [unreleased]

- present parsing errors by `bit status` and prevent tagging it until fixed
- fix detection of "export * from" syntax of ES6
- fix "Cannot read property 'lang' of null" error when resolving Vue dependencies

Expand Down
2 changes: 1 addition & 1 deletion e2e/commands/checkout.e2e.js
Expand Up @@ -89,7 +89,7 @@ describe('bit checkout command', function () {
});
describe('trying to tag when using an old version', () => {
before(() => {
helper.createComponentBarFoo('modified barFoo');
helper.createComponentBarFoo('console.log("modified components");');
});
it('should throw an error NewerVersionFound', () => {
const commitFunc = () => helper.commitComponent('bar/foo');
Expand Down
31 changes: 16 additions & 15 deletions e2e/commands/commit.e2e.js
Expand Up @@ -178,22 +178,22 @@ describe('bit tag command', function () {
expect(listOutput).to.deep.include({ id: 'components/b', localVersion: '0.0.1', currentVersion: '0.0.1' });
});
it('Should increment the patch version when no version type specified', () => {
helper.createFile('components', 'a.js', 'v0.0.2');
helper.createFile('components', 'b.js', 'v0.0.2');
helper.createFile('components', 'a.js', 'console.log("v0.0.2")');
helper.createFile('components', 'b.js', 'console.log("v0.0.2")');
output = helper.commitAllComponents('message');
expect(output).to.have.string('components/a@0.0.2');
expect(output).to.have.string('components/b@0.0.2');
});
it('Should increment the patch version when --patch flag specified', () => {
helper.createFile('components', 'a.js', 'v0.0.3');
helper.createFile('components', 'b.js', 'v0.0.3');
helper.createFile('components', 'a.js', 'console.log("v0.0.3")');
helper.createFile('components', 'b.js', 'console.log("v0.0.3")');
output = helper.commitAllComponents('message', '--patch');
expect(output).to.have.string('components/a@0.0.3');
expect(output).to.have.string('components/b@0.0.3');
});
it('Should increment the default version without the -m flag', () => {
helper.createFile('components', 'a.js', 'v0.0.4');
helper.createFile('components', 'b.js', 'v0.0.4');
helper.createFile('components', 'a.js', 'console.log("v0.0.4")');
helper.createFile('components', 'b.js', 'console.log("v0.0.4")');
output = helper.tagAllWithoutMessage();
expect(output).to.have.string('components/a@0.0.4');
expect(output).to.have.string('components/b@0.0.4');
Expand All @@ -204,15 +204,15 @@ describe('bit tag command', function () {
expect(tagWithoutChanges).to.have.string('nothing to tag');
});
it('Should increment the minor version when --minor flag specified', () => {
helper.createFile('components', 'a.js', 'v0.1.0');
helper.createFile('components', 'b.js', 'v0.1.0');
helper.createFile('components', 'a.js', 'console.log("v0.1.0")');
helper.createFile('components', 'b.js', 'console.log("v0.1.0")');
output = helper.commitAllComponents('message', '-f --minor');
expect(output).to.have.string('components/a@0.1.0');
expect(output).to.have.string('components/b@0.1.0');
});
it('Should increment the major version when --major flag specified', () => {
helper.createFile('components', 'a.js', 'v1.0.0');
helper.createFile('components', 'b.js', 'v1.0.0');
helper.createFile('components', 'a.js', 'console.log("v1.0.0")');
helper.createFile('components', 'b.js', 'console.log("v1.0.0")');
output = helper.commitAllComponents('message', '--major');
expect(output).to.have.string('components/a@1.0.0');
expect(output).to.have.string('components/b@1.0.0');
Expand All @@ -226,17 +226,17 @@ describe('bit tag command', function () {
expect(output).to.have.string('components/d@5.12.10');
});
it('Should set the exact version when specified on existing component', () => {
helper.createFile('components', 'a.js', 'v3.3.3');
helper.createFile('components', 'b.js', 'v3.3.3');
helper.createFile('components', 'a.js', 'console.log("v3.3.3")');
helper.createFile('components', 'b.js', 'console.log("v3.3.3")');
output = helper.commitAllComponents('message', '-f', '3.3.3');
expect(output).to.have.string('components/a@3.3.3');
expect(output).to.have.string('components/b@3.3.3');
});
it('Should throw error when the version already exists in one of the components', () => {
helper.createFile('components', 'a.js', 'v4.3.4');
helper.createFile('components', 'b.js', 'v4.3.4');
helper.createFile('components', 'a.js', 'console.log("v4.3.4")');
helper.createFile('components', 'b.js', 'console.log("v4.3.4")');
helper.commitComponent('components/a 4.3.4', 'message');
helper.createFile('components', 'a.js', 'v4.3.4 sss');
helper.createFile('components', 'a.js', 'console.log("v4.3.4 ssss")');
const tagWithExisting = () => helper.commitAllComponents('message', '', '4.3.4');
expect(tagWithExisting).to.throw('error: version 4.3.4 already exists for components/a');
});
Expand Down Expand Up @@ -359,6 +359,7 @@ describe('bit tag command', function () {
expect(output).to.not.have.string('failing test should fail');
});
});
// @todo: fix this test to not rely on all the "it" of 'tag all components'
describe('tagging all with --skip-tests flag', () => {
let output;
before(() => {
Expand Down
15 changes: 15 additions & 0 deletions e2e/commands/merge.e2e.js
Expand Up @@ -222,10 +222,25 @@ describe('bit merge command', function () {
it('should indicate that there are conflicts', () => {
expect(output).to.have.string(FileStatusWithoutChalk.manual);
});
it('bit status should indicate that there are issues with the file', () => {
const statusOutput = helper.runCmd('bit status');
expect(statusOutput).to.have.string('error found while parsing the file');
expect(statusOutput).to.have.string('bar/foo.js');
expect(statusOutput).to.have.string('Unexpected token');
});
it('should not be able to run the app because of the conflicts', () => {
const result = helper.runWithTryCatch('node app.js');
expect(result).to.have.string('SyntaxError: Unexpected token <<');
});
it('bit tag should not tag the component', () => {
const tagOutput = helper.runWithTryCatch('bit tag -a');
expect(tagOutput).to.have.string('error: issues found with the following component dependencies');
expect(tagOutput).to.have.string('error found while parsing the file');
});
it('bit tag should tag the component when --ignore-missing-dependencies flag is used', () => {
const tagOutput = helper.tagAllWithoutMessage('--ignore-missing-dependencies');
expect(tagOutput).to.have.string('1 components tagged');
});
});
describe('when using --ours flag', () => {
let output;
Expand Down
3 changes: 2 additions & 1 deletion e2e/flows/dist-outside-components.e2e.js
Expand Up @@ -2,6 +2,7 @@ import chai, { expect } from 'chai';
import fs from 'fs-extra';
import path from 'path';
import Helper from '../e2e-helper';
import { statusFailureMsg } from '../../src/cli/commands/public-cmds/status-cmd';

chai.use(require('chai-fs'));

Expand Down Expand Up @@ -375,7 +376,7 @@ export default function foo() { return isString() + ' and got foo'; };`;
});
it('should not indicate that missing dependencies', () => {
const status = helper.runCmd('bit status');
expect(status).to.not.have.string('missing');
expect(status).to.not.have.string(statusFailureMsg);
expect(status).to.not.have.string('modified');
});
describe('"bit build" after updating the imported component', () => {
Expand Down
5 changes: 3 additions & 2 deletions e2e/flows/es6-link-files.e2e.js
Expand Up @@ -2,6 +2,7 @@ import { expect } from 'chai';
import fs from 'fs-extra';
import path from 'path';
import Helper from '../e2e-helper';
import { statusFailureMsg } from '../../src/cli/commands/public-cmds/status-cmd';

describe('es6 components with link files', function () {
this.timeout(0);
Expand Down Expand Up @@ -41,7 +42,7 @@ describe('es6 components with link files', function () {
it('should not consider that index file as a dependency', () => {
output = helper.runCmd('bit status');
expect(output).to.have.string('bar/foo ... ok');
expect(output).to.not.have.string('missing dependencies');
expect(output).to.not.have.string(statusFailureMsg);
});
});

Expand Down Expand Up @@ -70,7 +71,7 @@ describe('es6 components with link files', function () {
it('should not consider both index files as a dependencies', () => {
output = helper.runCmd('bit status');
expect(output).to.have.string('bar/foo ... ok');
expect(output).to.not.have.string('missing dependencies');
expect(output).to.not.have.string(statusFailureMsg);
});
});

Expand Down
3 changes: 2 additions & 1 deletion e2e/flows/typescript-link-files.e2e.js
Expand Up @@ -2,6 +2,7 @@ import { expect } from 'chai';
import fs from 'fs-extra';
import path from 'path';
import Helper from '../e2e-helper';
import { statusFailureMsg } from '../../src/cli/commands/public-cmds/status-cmd';

describe('typescript components with link files', function () {
this.timeout(0);
Expand Down Expand Up @@ -41,7 +42,7 @@ describe('typescript components with link files', function () {
it('should not consider that index file as a dependency', () => {
output = helper.runCmd('bit status');
expect(output.includes('bar/foo ... ok')).to.be.true;
expect(output.includes('missing dependencies')).to.be.false;
expect(output.includes(statusFailureMsg)).to.be.false;
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/api/consumer/lib/status.js
Expand Up @@ -36,7 +36,7 @@ export default (async function status(): Promise<StatusResult> {
// If there is at least one we won't commit anything
const newAndModified = newComponents.concat(modifiedComponent);
const componentsWithMissingDeps = newAndModified.filter((component: Component) => {
return Boolean(component.missingDependencies);
return Boolean(component.issues);
});
Analytics.setExtraData('new_components', newComponents.length);
Analytics.setExtraData('staged_components', stagedComponents.length);
Expand Down
23 changes: 13 additions & 10 deletions src/cli/commands/public-cmds/status-cmd.js
Expand Up @@ -7,13 +7,16 @@ import type { StatusResult } from '../../../api/consumer/lib/status';
import Component from '../../../consumer/component';
import { immutableUnshift, isString } from '../../../utils';
import { formatBitString, formatNewBit } from '../../chalk-box';
import { missingDependenciesLabels } from '../../templates/missing-dependencies-template';
import { componentIssuesLabels, componentIssueToString } from '../../templates/component-issues-template';
import { Analytics } from '../../../analytics/analytics';
import { BASE_DOCS_DOMAIN } from '../../../constants';

const TROUBLESHOOTING_MESSAGE = `${chalk.yellow(
`see troubleshooting at https://${BASE_DOCS_DOMAIN}/docs/troubleshooting-isolating.html`
)}`;

export const statusFailureMsg = 'issues found';

export default class Status extends Command {
name = 'status';
description = `show the working area component(s) status.\n https://${BASE_DOCS_DOMAIN}/docs/cli-status.html`;
Expand Down Expand Up @@ -41,22 +44,22 @@ export default class Status extends Command {
let showTroubleshootingLink = false;

function formatMissing(missingComponent: Component) {
function formatMissingStr(key, array, label) {
if (!array || R.isEmpty(array)) return '';
function formatMissingStr(key, value, label) {
if (!value || R.isEmpty(value)) return '';
return (
chalk.yellow(`\n ${label}: \n`) +
chalk.white(
Object.keys(array)
.map(key => ` ${key} -> ${array[key].join(', ')}`)
Object.keys(value)
.map(key => ` ${key} -> ${componentIssueToString(value[key])}`)
.join('\n')
)
);
}

const missingStr = Object.keys(missingDependenciesLabels)
const missingStr = Object.keys(componentIssuesLabels)
.map((key) => {
if (missingComponent.missingDependencies[key]) Analytics.incExtraDataKey(key);
return formatMissingStr(key, missingComponent.missingDependencies[key], missingDependenciesLabels[key]);
if (missingComponent.issues[key]) Analytics.incExtraDataKey(key);
return formatMissingStr(key, missingComponent.issues[key], componentIssuesLabels[key]);
})
.join('');
return ` ${missingStr}\n`;
Expand All @@ -77,7 +80,7 @@ export default class Status extends Command {
bitFormatted += ' ... ';
if (!missing) return `${bitFormatted}${chalk.green('ok')}`;
showTroubleshootingLink = true;
return `${bitFormatted} ${chalk.red('missing dependencies')}${formatMissing(missing)}`;
return `${bitFormatted} ${chalk.red(statusFailureMsg)}${formatMissing(missing)}`;
}

const importPendingWarning = importPendingComponents.length
Expand All @@ -87,7 +90,7 @@ export default class Status extends Command {
: '';

const splitByMissing = R.groupBy((component) => {
return component.includes('missing dependencies') ? 'missing' : 'nonMissing';
return component.includes(statusFailureMsg) ? 'missing' : 'nonMissing';
});
const { missing, nonMissing } = splitByMissing(newComponents.map(c => format(c)));

Expand Down
4 changes: 2 additions & 2 deletions src/cli/default-error-handler.js
Expand Up @@ -50,7 +50,7 @@ import { MissingMainFile, MissingBitMapComponent, InvalidBitMap } from '../consu
import logger from '../logger/logger';
import RemoteUndefined from './commands/exceptions/remote-undefined';
import AddTestsWithoutId from './commands/exceptions/add-tests-without-id';
import missingDepsTemplate from './templates/missing-dependencies-template';
import componentIssuesTemplate from './templates/component-issues-template';
import newerVersionTemplate from './templates/newer-version-template';
import {
PathsNotExist,
Expand Down Expand Up @@ -216,7 +216,7 @@ once your changes are merged with the new remote version, please tag and export
[
MissingDependencies,
(err) => {
const missingDepsColored = missingDepsTemplate(err.components);
const missingDepsColored = componentIssuesTemplate(err.components);
return `error: issues found with the following component dependencies\n${missingDepsColored}`;
}
],
Expand Down
Expand Up @@ -2,7 +2,7 @@
import chalk from 'chalk';
import ConsumerComponent from '../../consumer/component/consumer-component';

export const missingDependenciesLabels = {
export const componentIssuesLabels = {
missingPackagesDependenciesOnFs:
'missing packages dependencies (use your package manager to make sure all package dependencies are installed)',
missingComponents:
Expand All @@ -11,10 +11,16 @@ export const missingDependenciesLabels = {
missingDependenciesOnFs: 'non-existing dependency files (please make sure all files exists on your workspace)',
missingLinks: 'missing links (use "bit link" to build missing component links)',
missingCustomModuleResolutionLinks: 'missing links (use "bit link" to build missing component links)',
relativeComponents: 'components with relative import statements (please use absolute paths for imported components)'
relativeComponents: 'components with relative import statements (please use absolute paths for imported components)',
parseErrors: 'error found while parsing the file (please edit the file and fix the parsing error)',
resolveErrors: 'error found while resolving the file dependencies (see the log for the full error)'
};

export default function missingDepsTemplate(components: ConsumerComponent[]) {
export function componentIssueToString(value: string[] | string) {
return Array.isArray(value) ? value.join(', ') : value;
}

export default function componentIssuesTemplate(components: ConsumerComponent[]) {
function format(missingComponent) {
return `${chalk.underline(chalk.cyan(missingComponent.id.toString()))}\n${formatMissing(missingComponent)}`;
}
Expand All @@ -24,20 +30,20 @@ export default function missingDepsTemplate(components: ConsumerComponent[]) {
}

function formatMissing(missingComponent: Object) {
function formatMissingStr(array, label) {
if (!array || array.length === 0) return '';
function formatMissingStr(value, label) {
if (!value || value.length === 0) return '';
return (
chalk.yellow(`${label}: \n`) +
chalk.white(
Object.keys(array)
.map(key => ` ${key} -> ${array[key].join(', ')}`)
Object.keys(value)
.map(key => ` ${key} -> ${componentIssueToString(value[key])}`)
.join('\n')
)
);
}

const missingStr = Object.keys(missingDependenciesLabels)
.map(key => formatMissingStr(missingComponent.missingDependencies[key], missingDependenciesLabels[key]))
const missingStr = Object.keys(componentIssuesLabels)
.map(key => formatMissingStr(missingComponent.issues[key], componentIssuesLabels[key]))
.join('');

return `${missingStr}\n`;
Expand Down
3 changes: 2 additions & 1 deletion src/consumer/component/consumer-component.js
Expand Up @@ -50,6 +50,7 @@ import AbstractBitJson from '../bit-json/abstract-bit-json';
import { Analytics } from '../../analytics/analytics';
import ConsumerComponent from '.';
import type { PackageJsonInstance } from './package-json';
import { componentIssuesLabels } from '../../cli/templates/component-issues-template';

export type customResolvedPath = { destinationPath: PathLinux, importSource: string };

Expand Down Expand Up @@ -115,7 +116,7 @@ export default class Component {
componentMap: ?ComponentMap; // always populated when the loadedFromFileSystem is true
componentFromModel: ?Component; // populated when loadedFromFileSystem is true and it exists in the model
isolatedEnvironment: IsolatedEnvironment;
missingDependencies: ?Object;
issues: { [label: $Keys<typeof componentIssuesLabels>]: { [fileName: string]: string[] | string } };
deprecated: boolean;
customResolvedPaths: customResolvedPath[];
_driver: Driver;
Expand Down

0 comments on commit 8280a79

Please sign in to comment.