Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: port parse to typescript #764

Merged
merged 6 commits into from Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 5 additions & 31 deletions @commitlint/parse/package.json
Expand Up @@ -3,35 +3,13 @@
"version": "8.1.0",
"description": "Lint your commit messages",
"main": "lib/index.js",
"types": "lib/index.d.ts",
"files": [
"lib/"
],
"scripts": {
"build": "cross-env NODE_ENV=production babel src --out-dir lib --source-maps",
"deps": "dep-check",
"pkg": "pkg-check",
"start": "concurrently \"ava -c 4 --verbose --watch\" \"yarn run watch\"",
"test": "ava -c 4 --verbose",
"watch": "babel src --out-dir lib --watch --source-maps"
},
"ava": {
"files": [
"src/**/*.test.js",
"!lib/**/*"
],
"source": [
"src/**/*.js",
"!lib/**/*"
],
"babel": "inherit",
"require": [
"babel-register"
]
},
"babel": {
"presets": [
"babel-preset-commitlint"
]
"pkg": "pkg-check"
},
"engines": {
"node": ">=4"
Expand All @@ -58,13 +36,9 @@
"devDependencies": {
"@commitlint/test": "8.0.0",
"@commitlint/utils": "^8.1.0",
"ava": "0.22.0",
"babel-cli": "6.26.0",
"babel-preset-commitlint": "^8.0.0",
"babel-register": "6.26.0",
"concurrently": "3.6.1",
"cross-env": "5.1.1",
"import-from": "3.0.0"
"@types/lodash": "^4.14.136",
"import-from": "3.0.0",
"typescript": "^3.5.3"
},
"dependencies": {
"conventional-changelog-angular": "^1.3.3",
Expand Down
17 changes: 0 additions & 17 deletions @commitlint/parse/src/index.js

This file was deleted.

@@ -1,40 +1,39 @@
import importFrom from 'import-from';
import test from 'ava';
import parse from '.';

test('throws when called without params', async t => {
const error = await t.throws(parse());
t.is(error.message, 'Expected a raw commit');
test('throws when called without params', () => {
expect(parse('')).rejects.toThrowError('Expected a raw commit');
byCedric marked this conversation as resolved.
Show resolved Hide resolved
});

test('throws when called with empty message', async t => {
const error = await t.throws(parse());
t.is(error.message, 'Expected a raw commit');
});

test('returns object with raw message', async t => {
test('returns object with raw message', async () => {
const message = 'type(scope): subject';
const actual = await parse(message);
t.is(actual.raw, message);

expect(actual).toHaveProperty('raw', message);
});

test('calls parser with message and passed options', async t => {
test('calls parser with message and passed options', async () => {
const message = 'message';

await parse(message, m => {
t.is(message, m);
return {};
});
expect.assertions(1);
await parse(
message,
(m: string): any => {
expect(m).toBe(message);
return {};
}
);
});

test('passes object up from parser function', async t => {
test('passes object up from parser function', async () => {
const message = 'message';
const result = {};
const result: any = {};
const actual = await parse(message, () => result);
t.is(actual, result);

expect(actual).toBe(result);
});

test('returns object with expected keys', async t => {
test('returns object with expected keys', async () => {
const message = 'message';
const actual = await parse(message);
const expected = {
Expand All @@ -51,10 +50,11 @@ test('returns object with expected keys', async t => {
subject: null,
type: null
};
t.deepEqual(actual, expected);

expect(actual).toMatchObject(expected);
});

test('uses angular grammar', async t => {
test('uses angular grammar', async () => {
const message = 'type(scope): subject';
const actual = await parse(message);
const expected = {
Expand All @@ -71,14 +71,15 @@ test('uses angular grammar', async t => {
subject: 'subject',
type: 'type'
};
t.deepEqual(actual, expected);

expect(actual).toMatchObject(expected);
});

test('uses custom opts parser', async t => {
test('uses custom opts parser', async () => {
const message = 'type(scope)-subject';
const changelogOpts = await importFrom(
process.cwd(),
'./fixtures/parser-preset/conventional-changelog-custom'
const changelogOpts: any = await importFrom(
__dirname,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bit weird because we are running Jest from the root of the monorepo process.cwd seems to be set to that folder. In this case, we need to scope it from within this package. This works, but not sure if this is the best solution.

Copy link
Contributor

@marionebl marionebl Jul 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine. We might want to use fixturez for this kind of thing to decouple test files from exact relative fixture paths. If you feel eager you could give it a try here, otherwise let's go forward as is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is important for @commitlint/load, was doing some work on that but did not finish in time. In this instance, it might be a bit of an overkill? 😬

'../fixtures/parser-preset/conventional-changelog-custom.js'
);
const actual = await parse(message, undefined, changelogOpts.parserOpts);
const expected = {
Expand All @@ -95,10 +96,11 @@ test('uses custom opts parser', async t => {
subject: 'subject',
type: 'type'
};
t.deepEqual(actual, expected);

expect(actual).toMatchObject(expected);
});

test('does not merge array properties with custom opts', async t => {
test('does not merge array properties with custom opts', async () => {
const message = 'type: subject';
const actual = await parse(message, undefined, {
headerPattern: /^(.*):\s(.*)$/,
Expand All @@ -117,73 +119,79 @@ test('does not merge array properties with custom opts', async t => {
subject: 'subject',
type: 'type'
};
t.deepEqual(actual, expected);

expect(actual).toMatchObject(expected);
});

test('supports scopes with /', async t => {
test('supports scopes with /', async () => {
const message = 'type(some/scope): subject';
const actual = await parse(message);
t.is(actual.scope, 'some/scope');
t.is(actual.subject, 'subject');

expect(actual.scope).toBe('some/scope');
expect(actual.subject).toBe('subject');
});

test('supports scopes with / and empty parserOpts', async t => {
test('supports scopes with / and empty parserOpts', async () => {
const message = 'type(some/scope): subject';
const actual = await parse(message, undefined, {});
t.is(actual.scope, 'some/scope');
t.is(actual.subject, 'subject');

expect(actual.scope).toBe('some/scope');
expect(actual.subject).toBe('subject');
});

test('ignores comments', async t => {
test('ignores comments', async () => {
const message = 'type(some/scope): subject\n# some comment';
const changelogOpts = await importFrom(
const changelogOpts: any = await importFrom(
process.cwd(),
'conventional-changelog-angular'
);
const opts = Object.assign({}, changelogOpts.parserOpts, {
commentChar: '#'
});
const actual = await parse(message, undefined, opts);
t.is(actual.body, null);
t.is(actual.footer, null);
t.is(actual.subject, 'subject');

expect(actual.body).toBe(null);
expect(actual.footer).toBe(null);
expect(actual.subject).toBe('subject');
});

test('registers inline #', async t => {
test('registers inline #', async () => {
const message =
'type(some/scope): subject #reference\n# some comment\nthings #reference';
const changelogOpts = await importFrom(
const changelogOpts: any = await importFrom(
process.cwd(),
'conventional-changelog-angular'
);
const opts = Object.assign({}, changelogOpts.parserOpts, {
commentChar: '#'
});
const actual = await parse(message, undefined, opts);
t.is(actual.subject, 'subject #reference');
t.is(actual.body, 'things #reference');

expect(actual.subject).toBe('subject #reference');
expect(actual.body).toBe('things #reference');
});

test('parses references leading subject', async t => {
test('parses references leading subject', async () => {
const message = '#1 some subject';
const opts = await importFrom(
process.cwd(),
'conventional-changelog-angular'
);
const {
references: [actual]
} = await parse(message, undefined, opts);
t.is(actual.issue, '1');
} = await parse(message, undefined, opts as any);

expect(actual.issue).toBe('1');
});

test('parses custom references', async t => {
test('parses custom references', async () => {
const message = '#1 some subject PREFIX-2';
const {references} = await parse(message, undefined, {
issuePrefixes: ['PREFIX-']
});

t.falsy(references.find(ref => ref.issue === '1'));
t.deepEqual(references.find(ref => ref.issue === '2'), {
expect(references.find(ref => ref.issue === '1')).toBeFalsy();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't add types to the parser output, you can't use ref => ref.issue === '1' like this. You need to add some types for this, everytime you use it. That's why I added it in the second commit. So this has to change to:

expect(references.find((ref: any) => ref.issue === '1')).toBeFalsy();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an optional type parameter defaulting to unknown should do the trick. Further discussion at the signature of parse

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand this fully, do you want to add a different typing to the existing references property from interface Commit? Or did you mean something else? 😄

expect(references.find(ref => ref.issue === '2')).toMatchObject({
action: null,
issue: '2',
owner: null,
Expand All @@ -193,44 +201,44 @@ test('parses custom references', async t => {
});
});

test('uses permissive default regex without parser opts', async t => {
test('uses permissive default regex without parser opts', async () => {
const message = 'chore(component,demo): bump';
const actual = await parse(message);

t.is(actual.scope, 'component,demo');
expect(actual.scope).toBe('component,demo');
});

test('uses permissive default regex with other parser opts', async t => {
test('uses permissive default regex with other parser opts', async () => {
const message = 'chore(component,demo): bump';
const actual = await parse(message, undefined, {commentChar: '#'});

t.is(actual.scope, 'component,demo');
expect(actual.scope).toBe('component,demo');
});

test('uses restrictive default regex in passed parser opts', async t => {
test('uses restrictive default regex in passed parser opts', async () => {
const message = 'chore(component,demo): bump';
const actual = await parse(message, undefined, {
headerPattern: /^(\w*)(?:\(([a-z]*)\))?: (.*)$/
});

t.is(actual.subject, null);
t.is(actual.scope, null);
expect(actual.subject).toBe(null);
expect(actual.scope).toBe(null);
});

test('works with chinese scope by default', async t => {
test('works with chinese scope by default', async () => {
const message = 'fix(面试评价): 测试';
const actual = await parse(message, undefined, {commentChar: '#'});

t.not(actual.subject, null);
t.not(actual.scope, null);
expect(actual.subject).not.toBe(null);
expect(actual.scope).not.toBe(null);
});

test('does not work with chinese scopes with incompatible pattern', async t => {
test('does not work with chinese scopes with incompatible pattern', async () => {
const message = 'fix(面试评价): 测试';
const actual = await parse(message, undefined, {
headerPattern: /^(\w*)(?:\(([a-z]*)\))?: (.*)$/
});

t.is(actual.subject, null);
t.is(actual.scope, null);
expect(actual.subject).toBe(null);
expect(actual.scope).toBe(null);
});
22 changes: 22 additions & 0 deletions @commitlint/parse/src/index.ts
@@ -0,0 +1,22 @@
import {isArray, mergeWith} from 'lodash';
import {Commit, Parser, ParserOptions} from './types';

const {sync} = require('conventional-commits-parser');
const defaultChangelogOpts = require('conventional-changelog-angular');

export default parse;
export * from './types';

async function parse(
message: string,
parser: Parser = sync,
parserOpts?: ParserOptions
): Promise<Commit> {
const defaultOpts = (await defaultChangelogOpts).parserOpts;
const opts = mergeWith({}, defaultOpts, parserOpts, (objValue, srcValue) => {
if (isArray(objValue)) return srcValue;
});
const parsed = parser(message, opts) as Commit;
parsed.raw = message;
return parsed;
}