Skip to content

Commit 8a8384f

Browse files
armano2armano2
authored
feat: simplify config resolution (#2398)
* feat: basic user config validation * fix: simplify config resolution and fix issue #327 * fix: remove no longer needed function * fix: disable some unwanted validations * fix: improve config validation * fix: remove redundant validation * fix: use reduceRight instead of reverse * fix: rollback some code * fix: drop invalid type casts * fix: rollback unnecessary changes * fix: rollback config validation * fix: add missing type-guards and restore order * fix: one more order change * fix: add one more missing type guard * fix: remove unused types reference * fix: add additional unit tests * fix: add additional regression tests - remove also unnecessary type check * fix: remove more unnecessary code changes * fix: correct order of merging plugins * fix: add missing type check * fix: remove invalid type check * fix: remove redundant code * fix: rollback some unnecessary changes * fix: optimize loadParserOpts
1 parent c33d493 commit 8a8384f

File tree

8 files changed

+322
-126
lines changed

8 files changed

+322
-126
lines changed

Diff for: ‎@commitlint/cli/src/cli.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ function getSeed(flags: CliFlags): Seed {
373373
: {parserPreset: flags['parser-preset']};
374374
}
375375

376-
function selectParserOpts(parserPreset: ParserPreset) {
376+
function selectParserOpts(parserPreset: ParserPreset | undefined) {
377377
if (typeof parserPreset !== 'object') {
378378
return undefined;
379379
}

Diff for: ‎@commitlint/load/src/load.test.ts

+26-18
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ test('extends-empty should have no rules', async () => {
2121
const actual = await load({}, {cwd});
2222

2323
expect(actual.rules).toMatchObject({});
24+
expect(actual.parserPreset).not.toBeDefined();
2425
});
2526

2627
test('uses seed as configured', async () => {
@@ -127,8 +128,9 @@ test('uses seed with parserPreset', async () => {
127128
{cwd}
128129
);
129130

130-
expect(actual.name).toBe('./conventional-changelog-custom');
131-
expect(actual.parserOpts).toMatchObject({
131+
expect(actual).toBeDefined();
132+
expect(actual!.name).toBe('./conventional-changelog-custom');
133+
expect(actual!.parserOpts).toMatchObject({
132134
headerPattern: /^(\w*)(?:\((.*)\))?-(.*)$/,
133135
});
134136
});
@@ -268,8 +270,9 @@ test('parser preset overwrites completely instead of merging', async () => {
268270
const cwd = await gitBootstrap('fixtures/parser-preset-override');
269271
const actual = await load({}, {cwd});
270272

271-
expect(actual.parserPreset.name).toBe('./custom');
272-
expect(actual.parserPreset.parserOpts).toMatchObject({
273+
expect(actual.parserPreset).toBeDefined();
274+
expect(actual.parserPreset!.name).toBe('./custom');
275+
expect(actual.parserPreset!.parserOpts).toMatchObject({
273276
headerPattern: /.*/,
274277
});
275278
});
@@ -278,8 +281,9 @@ test('recursive extends with parserPreset', async () => {
278281
const cwd = await gitBootstrap('fixtures/recursive-parser-preset');
279282
const actual = await load({}, {cwd});
280283

281-
expect(actual.parserPreset.name).toBe('./conventional-changelog-custom');
282-
expect(actual.parserPreset.parserOpts).toMatchObject({
284+
expect(actual.parserPreset).toBeDefined();
285+
expect(actual.parserPreset!.name).toBe('./conventional-changelog-custom');
286+
expect(actual.parserPreset!.parserOpts).toMatchObject({
283287
headerPattern: /^(\w*)(?:\((.*)\))?-(.*)$/,
284288
});
285289
});
@@ -402,11 +406,12 @@ test('resolves parser preset from conventional commits', async () => {
402406
const cwd = await npmBootstrap('fixtures/parser-preset-conventionalcommits');
403407
const actual = await load({}, {cwd});
404408

405-
expect(actual.parserPreset.name).toBe(
409+
expect(actual.parserPreset).toBeDefined();
410+
expect(actual.parserPreset!.name).toBe(
406411
'conventional-changelog-conventionalcommits'
407412
);
408-
expect(typeof actual.parserPreset.parserOpts).toBe('object');
409-
expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual(
413+
expect(typeof actual.parserPreset!.parserOpts).toBe('object');
414+
expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual(
410415
/^(\w*)(?:\((.*)\))?!?: (.*)$/
411416
);
412417
});
@@ -415,9 +420,10 @@ test('resolves parser preset from conventional angular', async () => {
415420
const cwd = await npmBootstrap('fixtures/parser-preset-angular');
416421
const actual = await load({}, {cwd});
417422

418-
expect(actual.parserPreset.name).toBe('conventional-changelog-angular');
419-
expect(typeof actual.parserPreset.parserOpts).toBe('object');
420-
expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual(
423+
expect(actual.parserPreset).toBeDefined();
424+
expect(actual.parserPreset!.name).toBe('conventional-changelog-angular');
425+
expect(typeof actual.parserPreset!.parserOpts).toBe('object');
426+
expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual(
421427
/^(\w*)(?:\((.*)\))?: (.*)$/
422428
);
423429
});
@@ -432,9 +438,10 @@ test('recursive resolves parser preset from conventional atom', async () => {
432438

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

435-
expect(actual.parserPreset.name).toBe('conventional-changelog-atom');
436-
expect(typeof actual.parserPreset.parserOpts).toBe('object');
437-
expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual(
441+
expect(actual.parserPreset).toBeDefined();
442+
expect(actual.parserPreset!.name).toBe('conventional-changelog-atom');
443+
expect(typeof actual.parserPreset!.parserOpts).toBe('object');
444+
expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual(
438445
/^(:.*?:) (.*)$/
439446
);
440447
});
@@ -445,11 +452,12 @@ test('resolves parser preset from conventional commits without factory support',
445452
);
446453
const actual = await load({}, {cwd});
447454

448-
expect(actual.parserPreset.name).toBe(
455+
expect(actual.parserPreset).toBeDefined();
456+
expect(actual.parserPreset!.name).toBe(
449457
'conventional-changelog-conventionalcommits'
450458
);
451-
expect(typeof actual.parserPreset.parserOpts).toBe('object');
452-
expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual(
459+
expect(typeof actual.parserPreset!.parserOpts).toBe('object');
460+
expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual(
453461
/^(\w*)(?:\((.*)\))?!?: (.*)$/
454462
);
455463
});

Diff for: ‎@commitlint/load/src/load.ts

+41-59
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,21 @@ import executeRule from '@commitlint/execute-rule';
22
import resolveExtends from '@commitlint/resolve-extends';
33
import {
44
LoadOptions,
5-
ParserPreset,
65
QualifiedConfig,
76
QualifiedRules,
7+
PluginRecords,
88
UserConfig,
9-
UserPreset,
109
} from '@commitlint/types';
1110
import isPlainObject from 'lodash/isPlainObject';
1211
import merge from 'lodash/merge';
13-
import mergeWith from 'lodash/mergeWith';
14-
import pick from 'lodash/pick';
15-
import union from 'lodash/union';
12+
import uniq from 'lodash/uniq';
1613
import Path from 'path';
1714
import resolveFrom from 'resolve-from';
1815
import {loadConfig} from './utils/load-config';
1916
import {loadParserOpts} from './utils/load-parser-opts';
2017
import loadPlugin from './utils/load-plugin';
2118
import {pickConfig} from './utils/pick-config';
2219

23-
const w = <T>(_: unknown, b: ArrayLike<T> | null | undefined | false) =>
24-
Array.isArray(b) ? b : undefined;
25-
2620
export default async function load(
2721
seed: UserConfig = {},
2822
options: LoadOptions = {}
@@ -35,11 +29,16 @@ export default async function load(
3529
// Might amount to breaking changes, defer until 9.0.0
3630

3731
// Merge passed config with file based options
38-
const config = pickConfig(merge({}, loaded ? loaded.config : null, seed));
39-
40-
const opts = merge(
41-
{extends: [], rules: {}, formatter: '@commitlint/format'},
42-
pick(config, 'extends', 'plugins', 'ignores', 'defaultIgnores')
32+
const config = pickConfig(
33+
merge(
34+
{
35+
extends: [],
36+
plugins: [],
37+
rules: {},
38+
},
39+
loaded ? loaded.config : null,
40+
seed
41+
)
4342
);
4443

4544
// Resolve parserPreset key
@@ -54,59 +53,35 @@ export default async function load(
5453
}
5554

5655
// Resolve extends key
57-
const extended = resolveExtends(opts, {
56+
const extended = resolveExtends(config, {
5857
prefix: 'commitlint-config',
5958
cwd: base,
6059
parserPreset: config.parserPreset,
61-
});
62-
63-
const preset = pickConfig(
64-
mergeWith(extended, config, w)
65-
) as unknown as UserPreset;
66-
preset.plugins = {};
67-
68-
// TODO: check if this is still necessary with the new factory based conventional changelog parsers
69-
// config.extends = Array.isArray(config.extends) ? config.extends : [];
60+
}) as unknown as UserConfig;
7061

71-
// Resolve parser-opts from preset
72-
if (typeof preset.parserPreset === 'object') {
73-
preset.parserPreset.parserOpts = await loadParserOpts(
74-
preset.parserPreset.name,
75-
// TODO: fix the types for factory based conventional changelog parsers
76-
preset.parserPreset as any
77-
);
62+
if (!extended.formatter || typeof extended.formatter !== 'string') {
63+
extended.formatter = '@commitlint/format';
7864
}
7965

80-
// Resolve config-relative formatter module
81-
if (typeof config.formatter === 'string') {
82-
preset.formatter =
83-
resolveFrom.silent(base, config.formatter) || config.formatter;
84-
}
85-
86-
// Read plugins from extends
66+
let plugins: PluginRecords = {};
8767
if (Array.isArray(extended.plugins)) {
88-
config.plugins = union(config.plugins, extended.plugins || []);
89-
}
90-
91-
// resolve plugins
92-
if (Array.isArray(config.plugins)) {
93-
config.plugins.forEach((plugin) => {
68+
uniq(extended.plugins || []).forEach((plugin) => {
9469
if (typeof plugin === 'string') {
95-
loadPlugin(preset.plugins, plugin, process.env.DEBUG === 'true');
70+
plugins = loadPlugin(plugins, plugin, process.env.DEBUG === 'true');
9671
} else {
97-
preset.plugins.local = plugin;
72+
plugins.local = plugin;
9873
}
9974
});
10075
}
10176

102-
const rules = preset.rules ? preset.rules : {};
103-
const qualifiedRules = (
77+
const rules = (
10478
await Promise.all(
105-
Object.entries(rules || {}).map((entry) => executeRule<any>(entry))
79+
Object.entries(extended.rules || {}).map((entry) => executeRule(entry))
10680
)
10781
).reduce<QualifiedRules>((registry, item) => {
108-
const [key, value] = item as any;
109-
(registry as any)[key] = value;
82+
// type of `item` can be null, but Object.entries always returns key pair
83+
const [key, value] = item!;
84+
registry[key] = value;
11085
return registry;
11186
}, {});
11287

@@ -118,17 +93,24 @@ export default async function load(
11893
: 'https://github.com/conventional-changelog/commitlint/#what-is-commitlint';
11994

12095
const prompt =
121-
preset.prompt && isPlainObject(preset.prompt) ? preset.prompt : {};
96+
extended.prompt && isPlainObject(extended.prompt) ? extended.prompt : {};
12297

12398
return {
124-
extends: preset.extends!,
125-
formatter: preset.formatter!,
126-
parserPreset: preset.parserPreset! as ParserPreset,
127-
ignores: preset.ignores!,
128-
defaultIgnores: preset.defaultIgnores!,
129-
plugins: preset.plugins!,
130-
rules: qualifiedRules,
131-
helpUrl,
99+
extends: Array.isArray(extended.extends)
100+
? extended.extends
101+
: typeof extended.extends === 'string'
102+
? [extended.extends]
103+
: [],
104+
// Resolve config-relative formatter module
105+
formatter:
106+
resolveFrom.silent(base, extended.formatter) || extended.formatter,
107+
// Resolve parser-opts from preset
108+
parserPreset: await loadParserOpts(extended.parserPreset),
109+
ignores: extended.ignores,
110+
defaultIgnores: extended.defaultIgnores,
111+
plugins: plugins,
112+
rules: rules,
113+
helpUrl: helpUrl,
132114
prompt,
133115
};
134116
}

Diff for: ‎@commitlint/load/src/utils/load-parser-opts.ts

+50-27
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,71 @@
1+
import {ParserPreset} from '@commitlint/types';
2+
3+
function isObjectLike(obj: unknown): obj is Record<string, unknown> {
4+
return Boolean(obj) && typeof obj === 'object'; // typeof null === 'object'
5+
}
6+
7+
function isParserOptsFunction<T extends ParserPreset>(
8+
obj: T
9+
): obj is T & {
10+
parserOpts: (...args: any[]) => any;
11+
} {
12+
return typeof obj.parserOpts === 'function';
13+
}
14+
115
export async function loadParserOpts(
2-
parserName: string,
3-
pendingParser: Promise<any>
4-
) {
16+
pendingParser: string | ParserPreset | Promise<ParserPreset> | undefined
17+
): Promise<ParserPreset | undefined> {
18+
if (!pendingParser || typeof pendingParser !== 'object') {
19+
return undefined;
20+
}
521
// Await for the module, loaded with require
622
const parser = await pendingParser;
723

8-
// Await parser opts if applicable
9-
if (
10-
typeof parser === 'object' &&
11-
typeof parser.parserOpts === 'object' &&
12-
typeof parser.parserOpts.then === 'function'
13-
) {
14-
return (await parser.parserOpts).parserOpts;
24+
// exit early, no opts to resolve
25+
if (!parser.parserOpts) {
26+
return parser;
27+
}
28+
29+
// Pull nested parserOpts, might happen if overwritten with a module in main config
30+
if (typeof parser.parserOpts === 'object') {
31+
// Await parser opts if applicable
32+
parser.parserOpts = await parser.parserOpts;
33+
if (
34+
isObjectLike(parser.parserOpts) &&
35+
isObjectLike(parser.parserOpts.parserOpts)
36+
) {
37+
parser.parserOpts = parser.parserOpts.parserOpts;
38+
}
39+
return parser;
1540
}
1641

1742
// Create parser opts from factory
1843
if (
19-
typeof parser === 'object' &&
20-
typeof parser.parserOpts === 'function' &&
21-
parserName.startsWith('conventional-changelog-')
44+
isParserOptsFunction(parser) &&
45+
typeof parser.name === 'string' &&
46+
parser.name.startsWith('conventional-changelog-')
2247
) {
23-
return await new Promise((resolve) => {
24-
const result = parser.parserOpts((_: never, opts: {parserOpts: any}) => {
25-
resolve(opts.parserOpts);
48+
return new Promise((resolve) => {
49+
const result = parser.parserOpts((_: never, opts: any) => {
50+
resolve({
51+
...parser,
52+
parserOpts: opts?.parserOpts,
53+
});
2654
});
2755

2856
// If result has data or a promise, the parser doesn't support factory-init
2957
// due to https://github.com/nodejs/promises-debugging/issues/16 it just quits, so let's use this fallback
3058
if (result) {
3159
Promise.resolve(result).then((opts) => {
32-
resolve(opts.parserOpts);
60+
resolve({
61+
...parser,
62+
parserOpts: opts?.parserOpts,
63+
});
3364
});
3465
}
66+
return;
3567
});
3668
}
3769

38-
// Pull nested paserOpts, might happen if overwritten with a module in main config
39-
if (
40-
typeof parser === 'object' &&
41-
typeof parser.parserOpts === 'object' &&
42-
typeof parser.parserOpts.parserOpts === 'object'
43-
) {
44-
return parser.parserOpts.parserOpts;
45-
}
46-
47-
return parser.parserOpts;
70+
return parser;
4871
}

Diff for: ‎@commitlint/load/src/utils/pick-config.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import {UserConfig} from '@commitlint/types';
21
import pick from 'lodash/pick';
32

4-
export const pickConfig = (input: unknown): UserConfig =>
3+
export const pickConfig = (input: unknown): Record<string, unknown> =>
54
pick(
65
input,
76
'extends',

0 commit comments

Comments
 (0)