fix: simplify config resolution and fix issue conventional-changelog#327

Author: armano2

@commitlint/load/src/load.ts

@@ -1,7 +1,7 @@
 import Path from 'path';
 
 import merge from 'lodash/merge';
-import union from 'lodash/union';
+import uniq from 'lodash/uniq';
 import resolveFrom from 'resolve-from';
 
 import executeRule from '@commitlint/execute-rule';
@@ -35,6 +35,8 @@ export default async function load(
 	const config = pickConfig(
 		merge(
 			{
+				extends: [],
+				plugins: [],
 				rules: {},
 				formatter: '@commitlint/format',
 				helpUrl:
@@ -57,7 +59,7 @@ export default async function load(
 	}
 
 	// Resolve extends key
-	const extended = resolveExtends(config, {
+	const extended = resolveExtends(config as any, {
 		prefix: 'commitlint-config',
 		cwd: base,
 		parserPreset: config.parserPreset,
@@ -66,13 +68,7 @@ export default async function load(
 	validateConfig(extended);
 
 	let plugins: PluginRecords = {};
-	// TODO: this object merging should be done in resolveExtends
-	union(
-		// Read plugins from config
-		Array.isArray(config.plugins) ? config.plugins : [],
-		// Read plugins from extends
-		Array.isArray(extended.plugins) ? extended.plugins : []
-	).forEach((plugin) => {
+	uniq(extended.plugins || []).forEach((plugin) => {
 		if (typeof plugin === 'string') {
 			plugins = loadPlugin(plugins, plugin, process.env.DEBUG === 'true');
 		} else {
@@ -82,10 +78,7 @@ export default async function load(
 
 	const rules = (
 		await Promise.all(
-			Object.entries({
-				...(typeof extended.rules === 'object' ? extended.rules || {} : {}),
-				...(typeof config.rules === 'object' ? config.rules || {} : {}),
-			}).map((entry) => executeRule(entry))
+			Object.entries(extended.rules || {}).map((entry) => executeRule(entry))
 		)
 	).reduce<QualifiedRules>((registry, item) => {
 		// type of `item` can be null, but Object.entries always returns key pair
@@ -111,9 +104,6 @@ export default async function load(
 		defaultIgnores: extended.defaultIgnores,
 		plugins: plugins,
 		rules: rules,
-		helpUrl:
-			typeof extended.helpUrl === 'string'
-				? extended.helpUrl
-				: 'https://github.com/conventional-changelog/commitlint/#what-is-commitlint',
+		helpUrl: extended.helpUrl,
 	};
 }

@commitlint/load/src/utils/validators.ts

@@ -1,3 +1,5 @@
+import {Plugin, RulesConfig} from '@commitlint/types';
+
 export function isObjectLike(obj: unknown): obj is Record<string, unknown> {
 	return Boolean(obj) && typeof obj === 'object'; // typeof null === 'object'
 }
@@ -25,22 +27,33 @@ export function validateConfig(
 	formatter: string;
 	ignores?: ((commit: string) => boolean)[];
 	defaultIgnores?: boolean;
+	plugins?: (Plugin | string)[];
+	rules: Partial<RulesConfig>;
+	helpUrl: string;
 	[key: string]: unknown;
 } {
 	if (!isObjectLike(config)) {
-		throw new Error('Invalid configuration, parserPreset must be an object');
+		throw new Error('Invalid configuration, `parserPreset` must be an object');
 	}
 	if (typeof config.formatter !== 'string') {
-		throw new Error('Invalid configuration, formatter must be a string');
+		throw new Error('Invalid configuration, `formatter` must be a string');
 	}
 	if (config.ignores && !Array.isArray(config.ignores)) {
-		throw new Error('Invalid configuration, ignores must ba an array');
+		throw new Error('Invalid configuration, `ignores` must ba an array');
+	}
+	if (config.plugins && !Array.isArray(config.plugins)) {
+		throw new Error('Invalid configuration, `plugins` must ba an array');
 	}
 	if (
 		typeof config.defaultIgnores !== 'boolean' &&
 		typeof config.defaultIgnores !== 'undefined'
 	) {
-		throw new Error('Invalid configuration, defaultIgnores must ba true/false');
+		throw new Error(
+			'Invalid configuration, `defaultIgnores` must ba true/false'
+		);
+	}
+	if (typeof config.helpUrl !== 'string') {
+		throw new Error('Invalid configuration, `helpUrl` must be a string');
 	}
 }
 

@commitlint/resolve-extends/src/index.test.ts

@@ -348,3 +348,50 @@ test('should fall back to conventional-changelog-lint-config prefix', () => {
 		},
 	});
 });
+
+// https://github.com/conventional-changelog/commitlint/issues/327
+test('parserPreset should resolve correctly in extended configuration', () => {
+	const input = {extends: ['extender-name'], zero: 'root'};
+
+	const require = (id: string) => {
+		switch (id) {
+			case 'extender-name':
+				return {
+					extends: ['recursive-extender-name'],
+					parserPreset: {
+						parserOpts: {
+							issuePrefixes: ['#', '!', '&', 'no-references'],
+							referenceActions: null,
+						},
+					},
+				};
+			case 'recursive-extender-name':
+				return {
+					parserPreset: {
+						parserOpts: {
+							issuePrefixes: ['#', '!'],
+						},
+					},
+				};
+			default:
+				return {};
+		}
+	};
+
+	const ctx = {resolve: id, require: jest.fn(require)} as ResolveExtendsContext;
+
+	const actual = resolveExtends(input, ctx);
+
+	const expected = {
+		extends: ['extender-name'],
+		parserPreset: {
+			parserOpts: {
+				issuePrefixes: ['#', '!', '&', 'no-references'],
+				referenceActions: null,
+			},
+		},
+		zero: 'root',
+	};
+
+	expect(actual).toEqual(expected);
+});

@commitlint/resolve-extends/src/index.ts

@@ -4,10 +4,14 @@ import 'resolve-global';
 import resolveFrom from 'resolve-from';
 import merge from 'lodash/merge';
 import mergeWith from 'lodash/mergeWith';
-import {UserConfig} from '@commitlint/types';
 
 const importFresh = require('import-fresh');
 
+export interface ResolveExtendsConfig {
+	extends?: string | string[];
+	[key: string]: unknown;
+}
+
 export interface ResolvedConfig {
 	parserPreset?: unknown;
 	[key: string]: unknown;
@@ -22,32 +26,45 @@ export interface ResolveExtendsContext {
 	require?<T>(id: string): T;
 }
 
+function mergeStrategy(objValue: unknown, srcValue: unknown, key: string) {
+	if (key === 'parserPreset') {
+		if (typeof srcValue !== 'object') {
+			return objValue;
+		}
+	} else if (key === 'rules') {
+		if (typeof objValue !== 'object') {
+			return srcValue;
+		}
+	} else if (key === 'plugins') {
+		if (!Array.isArray(objValue)) {
+			return srcValue;
+		}
+	} else if (Array.isArray(objValue)) {
+		return srcValue;
+	}
+}
+
 export default function resolveExtends(
-	config: UserConfig = {},
+	config: ResolveExtendsConfig = {},
 	context: ResolveExtendsContext = {}
-) {
+): ResolvedConfig {
 	const {extends: e} = config;
-	const extended = loadExtends(config, context).reduce(
-		(r, {extends: _, ...c}) =>
-			mergeWith(r, c, (objValue, srcValue) => {
-				if (Array.isArray(objValue)) {
-					return srcValue;
-				}
-			}),
+	const extended = loadExtends(config, context);
+	extended.push(config);
+	return extended.reduce(
+		(r, {extends: _, ...c}) => mergeWith(r, c, mergeStrategy),
 		e ? {extends: e} : {}
 	);
-
-	return merge({}, extended, config);
 }
 
 function loadExtends(
-	config: UserConfig = {},
+	config: ResolveExtendsConfig = {},
 	context: ResolveExtendsContext = {}
 ): ResolvedConfig[] {
 	const {extends: e} = config;
-	const ext = e ? (Array.isArray(e) ? e : [e]) : [];
+	const ext = e ? (Array.isArray(e) ? [...e] : [e]) : [];
 
-	return ext.reduce<ResolvedConfig[]>((configs, raw) => {
+	return ext.reverse().reduce<ResolvedConfig[]>((configs, raw) => {
 		const load = context.require || require;
 		const resolved = resolveConfig(raw, context);
 		const c = load(resolved);
@@ -73,7 +90,7 @@ function loadExtends(
 			config.parserPreset = parserPreset;
 		}
 
-		return [...configs, ...loadExtends(c, ctx), c];
+		return [...loadExtends(c, ctx), c, ...configs];
 	}, []);
 }