Skip to content

fix(load): add support for factory-callback parser presets #834

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

Merged
merged 3 commits into from
Oct 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
parserPreset: 'conventional-changelog-angular'
};
7 changes: 7 additions & 0 deletions @commitlint/load/fixtures/parser-preset-angular/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "parser-preset-angular",
"version": "1.0.0",
"devDependencies": {
"conventional-changelog-angular": "5.0.5"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
parserPreset: 'conventional-changelog-conventionalcommits'
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "parser-preset-conventionalcommits",
"version": "1.0.0",
"devDependencies": {
"conventional-changelog-conventionalcommits": "4.2.1"
}
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
parserPreset: 'conventional-changelog-atom'
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@second-extend/recursive-parser-preset-conventional-atom",
"version": "1.0.0",
"devDependencies": {
"conventional-changelog-atom": "2.0.3"
}
}

This file was deleted.

This file was deleted.

69 changes: 47 additions & 22 deletions @commitlint/load/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import path from 'path';
import executeRule from '@commitlint/execute-rule';
import resolveExtends from '@commitlint/resolve-extends';
import cosmiconfig from 'cosmiconfig';
import {toPairs, merge, mergeWith, pick} from 'lodash';
import {toPairs, merge, mergeWith, pick, startsWith} from 'lodash';
import resolveFrom from 'resolve-from';
import loadPlugin from './utils/loadPlugin';

Expand Down Expand Up @@ -30,20 +30,14 @@ export default async (seed = {}, options = {cwd: process.cwd()}) => {
pick(config, 'extends', 'plugins', 'ignores', 'defaultIgnores')
);

// Resolve parserPreset key from flat-non-extended config
// Resolve parserPreset key when overwritten by main config
if (typeof config.parserPreset === 'string') {
const resolvedParserPreset = resolveFrom(base, config.parserPreset);
let resolvedParserConfig = await require(resolvedParserPreset);

// Resolve loaded parser preset factory
if (typeof resolvedParserConfig === 'function') {
resolvedParserConfig = await resolvedParserConfig();
}

config.parserPreset = {
name: config.parserPreset,
path: resolvedParserPreset,
parserOpts: resolvedParserConfig.parserOpts
parserOpts: require(resolvedParserPreset)
Copy link
Member Author

Choose a reason for hiding this comment

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

The code before the first attempt was (await require(resolvedParserPreset)).parserOpts, which is not going to work with the newer callback-presets. I took a look at @commitlint/resolve-extended and it does exactly the same as written here, I think that's a better way-to-go compared to the instant-extraction of parserOpts (like the code I just mentioned).

};
}

Expand All @@ -55,20 +49,13 @@ export default async (seed = {}, options = {cwd: process.cwd()}) => {
});

const preset = valid(mergeWith(extended, config, w));
// Await parser-preset if applicable
if (
typeof preset.parserPreset === 'object' &&
typeof preset.parserPreset.parserOpts === 'object' &&
typeof preset.parserPreset.parserOpts.then === 'function'
) {
let parserPreset = await preset.parserPreset.parserOpts;

// Resolve loaded parser preset factory from extended config
if (typeof parserPreset === 'function') {
parserPreset = await parserPreset();
}

preset.parserPreset.parserOpts = parserPreset.parserOpts;
// Resolve parser-opts from preset
if (typeof preset.parserPreset === 'object') {
preset.parserPreset.parserOpts = await loadParserOpts(
preset.parserPreset.name,
preset.parserPreset
);
}

// Resolve config-relative formatter module
Expand Down Expand Up @@ -129,3 +116,41 @@ async function loadConfig(cwd, configPath) {

return {};
}

async function loadParserOpts(parserName, pendingParser) {
// Await for the module, loaded with require
const parser = await pendingParser;

// Await parser opts if applicable
if (
typeof parser === 'object' &&
typeof parser.parserOpts === 'object' &&
typeof parser.parserOpts.then === 'function'
) {
return (await parser.parserOpts).parserOpts;
}

// Create parser opts from factory
if (
typeof parser === 'object' &&
typeof parser.parserOpts === 'function' &&
startsWith(parserName, 'conventional-changelog-')
) {
return await new Promise(resolve => {
parser.parserOpts((_, opts) => {
resolve(opts.parserOpts);
});
});
}

// Pull nested paserOpts, might happen if overwritten with a module in main config
if (
typeof parser === 'object' &&
typeof parser.parserOpts === 'object' &&
typeof parser.parserOpts.parserOpts === 'object'
) {
return parser.parserOpts.parserOpts;
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 how this "happens", but I think it's related to the context-overwrite when calling @commitlint/resolve-extended. The failing test was the parser-preset-override, I don't think anything is wrong with that tests though.

What I think what happens:

  • commitlint.config.js overwrites parserPreset with a module
  • overwrite is picked up and module is loaded
  • due to this change, it's not using parserOpts but the parser preset itself.

}

return parser.parserOpts;
}
67 changes: 42 additions & 25 deletions @commitlint/load/src/index.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import path from 'path';
import {fix, git} from '@commitlint/test';
import {fix, git, npm} from '@commitlint/test';
import test from 'ava';
import execa from 'execa';
import resolveFrom from 'resolve-from';

import load from '.';
Expand Down Expand Up @@ -84,20 +85,6 @@ test('uses seed with parserPreset', async t => {
});
});

test('uses seed with parserPreset factory', async t => {
const cwd = await git.bootstrap('fixtures/parser-preset-factory');
const {parserPreset: actual} = await load(
{
parserPreset: './conventional-changelog-factory'
},
{cwd}
);
t.is(actual.name, './conventional-changelog-factory');
t.deepEqual(actual.parserOpts, {
headerPattern: /^(\w*)(?:\((.*)\))?-(.*)$/
});
});

test('invalid extend should throw', async t => {
const cwd = await git.bootstrap('fixtures/extends-invalid');
await t.throws(load({}, {cwd}));
Expand Down Expand Up @@ -228,16 +215,6 @@ test('recursive extends with parserPreset', async t => {
);
});

test('recursive extends with parserPreset factory', async t => {
const cwd = await git.bootstrap('fixtures/recursive-parser-preset-factory');
const actual = await load({}, {cwd});

t.is(actual.parserPreset.name, './conventional-changelog-factory');
t.deepEqual(actual.parserPreset.parserOpts, {
headerPattern: /^(\w*)(?:\((.*)\))?-(.*)$/
});
});

test('ignores unknow keys', async t => {
const cwd = await git.bootstrap('fixtures/trash-file');
const actual = await load({}, {cwd});
Expand Down Expand Up @@ -351,3 +328,43 @@ test('does not mutate config module reference', async t => {

t.is(before, after);
});

test('resolves parser preset from conventional commits', async t => {
const cwd = await npm.bootstrap('fixtures/parser-preset-conventionalcommits');
const actual = await load({}, {cwd});

t.is(actual.parserPreset.name, 'conventional-changelog-conventionalcommits');
t.is(typeof actual.parserPreset.parserOpts, 'object');
t.deepEqual(
actual.parserPreset.parserOpts.headerPattern,
/^(\w*)(?:\((.*)\))?!?: (.*)$/
);
});

test('resolves parser preset from conventional angular', async t => {
const cwd = await npm.bootstrap('fixtures/parser-preset-angular');
const actual = await load({}, {cwd});

t.is(actual.parserPreset.name, 'conventional-changelog-angular');
t.is(typeof actual.parserPreset.parserOpts, 'object');
t.deepEqual(
actual.parserPreset.parserOpts.headerPattern,
/^(\w*)(?:\((.*)\))?: (.*)$/
);
});

test('recursive resolves parser preset from conventional atom', async t => {
const cwd = await git.bootstrap(
'fixtures/recursive-parser-preset-conventional-atom'
);
// the package file is nested in 2 folders, `npm.bootstrap` cant do that
await execa('npm', ['install'], {
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 there is any other way, let me know 😄 I think this is not that bad, but I didn't see any other option for now.

cwd: path.resolve(cwd, 'first-extended', 'second-extended')
});

const actual = await load({}, {cwd});

t.is(actual.parserPreset.name, 'conventional-changelog-atom');
t.is(typeof actual.parserPreset.parserOpts, 'object');
t.deepEqual(actual.parserPreset.parserOpts.headerPattern, /^(:.*?:) (.*)$/);
});