Skip to content

refactor: port parse to typescript #764

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 11, 2019
Merged

refactor: port parse to typescript #764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
46596a3
refactor: port parse to typescript
byCedric Jul 27, 2019
0974153
refactor: add commit interface to parse return type
byCedric Jul 27, 2019
8cd3051
refactor: remove replaced dependencies from parse
byCedric Jul 27, 2019
8f08e67
refactor(parse): add strict options interface
byCedric Jul 29, 2019
c161bb0
refactor(parse): add strict parser type
byCedric Jul 29, 2019
7767d51
test(parse): add empty and no commit message tests
byCedric Jul 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 5 additions & 31 deletions @commitlint/parse/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop

This file was deleted.

136 changes: 74 additions & 62 deletions @commitlint/parse/src/index.test.js → @commitlint/parse/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,43 @@
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 as any)()).rejects.toThrowError('Expected a raw commit');
});

test('throws when called with empty message', async t => {
const error = await t.throws(parse());
t.is(error.message, 'Expected a raw commit');
test('throws when called with empty message', () => {
expect(parse('')).rejects.toThrowError('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 +54,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 +75,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 •
edited
Loading

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 +100,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 +123,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 +205,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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}
Loading