From 8a8384f3c18954447cb633e76a573e1db71a1440 Mon Sep 17 00:00:00 2001 From: Armano Date: Tue, 16 Nov 2021 03:34:47 +0100 Subject: [PATCH] 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 --- @commitlint/cli/src/cli.ts | 2 +- @commitlint/load/src/load.test.ts | 44 ++-- @commitlint/load/src/load.ts | 100 ++++----- .../load/src/utils/load-parser-opts.ts | 77 ++++--- @commitlint/load/src/utils/pick-config.ts | 3 +- @commitlint/resolve-extends/src/index.test.ts | 191 +++++++++++++++++- @commitlint/resolve-extends/src/index.ts | 17 +- @commitlint/types/src/load.ts | 14 +- 8 files changed, 322 insertions(+), 126 deletions(-) diff --git a/@commitlint/cli/src/cli.ts b/@commitlint/cli/src/cli.ts index 974974be8d..891c534f70 100644 --- a/@commitlint/cli/src/cli.ts +++ b/@commitlint/cli/src/cli.ts @@ -373,7 +373,7 @@ function getSeed(flags: CliFlags): Seed { : {parserPreset: flags['parser-preset']}; } -function selectParserOpts(parserPreset: ParserPreset) { +function selectParserOpts(parserPreset: ParserPreset | undefined) { if (typeof parserPreset !== 'object') { return undefined; } diff --git a/@commitlint/load/src/load.test.ts b/@commitlint/load/src/load.test.ts index 824c3e130e..3358080d25 100644 --- a/@commitlint/load/src/load.test.ts +++ b/@commitlint/load/src/load.test.ts @@ -21,6 +21,7 @@ test('extends-empty should have no rules', async () => { const actual = await load({}, {cwd}); expect(actual.rules).toMatchObject({}); + expect(actual.parserPreset).not.toBeDefined(); }); test('uses seed as configured', async () => { @@ -127,8 +128,9 @@ test('uses seed with parserPreset', async () => { {cwd} ); - expect(actual.name).toBe('./conventional-changelog-custom'); - expect(actual.parserOpts).toMatchObject({ + expect(actual).toBeDefined(); + expect(actual!.name).toBe('./conventional-changelog-custom'); + expect(actual!.parserOpts).toMatchObject({ headerPattern: /^(\w*)(?:\((.*)\))?-(.*)$/, }); }); @@ -268,8 +270,9 @@ test('parser preset overwrites completely instead of merging', async () => { const cwd = await gitBootstrap('fixtures/parser-preset-override'); const actual = await load({}, {cwd}); - expect(actual.parserPreset.name).toBe('./custom'); - expect(actual.parserPreset.parserOpts).toMatchObject({ + expect(actual.parserPreset).toBeDefined(); + expect(actual.parserPreset!.name).toBe('./custom'); + expect(actual.parserPreset!.parserOpts).toMatchObject({ headerPattern: /.*/, }); }); @@ -278,8 +281,9 @@ test('recursive extends with parserPreset', async () => { const cwd = await gitBootstrap('fixtures/recursive-parser-preset'); const actual = await load({}, {cwd}); - expect(actual.parserPreset.name).toBe('./conventional-changelog-custom'); - expect(actual.parserPreset.parserOpts).toMatchObject({ + expect(actual.parserPreset).toBeDefined(); + expect(actual.parserPreset!.name).toBe('./conventional-changelog-custom'); + expect(actual.parserPreset!.parserOpts).toMatchObject({ headerPattern: /^(\w*)(?:\((.*)\))?-(.*)$/, }); }); @@ -402,11 +406,12 @@ test('resolves parser preset from conventional commits', async () => { const cwd = await npmBootstrap('fixtures/parser-preset-conventionalcommits'); const actual = await load({}, {cwd}); - expect(actual.parserPreset.name).toBe( + expect(actual.parserPreset).toBeDefined(); + expect(actual.parserPreset!.name).toBe( 'conventional-changelog-conventionalcommits' ); - expect(typeof actual.parserPreset.parserOpts).toBe('object'); - expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual( + expect(typeof actual.parserPreset!.parserOpts).toBe('object'); + expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual( /^(\w*)(?:\((.*)\))?!?: (.*)$/ ); }); @@ -415,9 +420,10 @@ test('resolves parser preset from conventional angular', async () => { const cwd = await npmBootstrap('fixtures/parser-preset-angular'); const actual = await load({}, {cwd}); - expect(actual.parserPreset.name).toBe('conventional-changelog-angular'); - expect(typeof actual.parserPreset.parserOpts).toBe('object'); - expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual( + expect(actual.parserPreset).toBeDefined(); + expect(actual.parserPreset!.name).toBe('conventional-changelog-angular'); + expect(typeof actual.parserPreset!.parserOpts).toBe('object'); + expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual( /^(\w*)(?:\((.*)\))?: (.*)$/ ); }); @@ -432,9 +438,10 @@ test('recursive resolves parser preset from conventional atom', async () => { const actual = await load({}, {cwd}); - expect(actual.parserPreset.name).toBe('conventional-changelog-atom'); - expect(typeof actual.parserPreset.parserOpts).toBe('object'); - expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual( + expect(actual.parserPreset).toBeDefined(); + expect(actual.parserPreset!.name).toBe('conventional-changelog-atom'); + expect(typeof actual.parserPreset!.parserOpts).toBe('object'); + expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual( /^(:.*?:) (.*)$/ ); }); @@ -445,11 +452,12 @@ test('resolves parser preset from conventional commits without factory support', ); const actual = await load({}, {cwd}); - expect(actual.parserPreset.name).toBe( + expect(actual.parserPreset).toBeDefined(); + expect(actual.parserPreset!.name).toBe( 'conventional-changelog-conventionalcommits' ); - expect(typeof actual.parserPreset.parserOpts).toBe('object'); - expect((actual.parserPreset.parserOpts as any).headerPattern).toEqual( + expect(typeof actual.parserPreset!.parserOpts).toBe('object'); + expect((actual.parserPreset!.parserOpts as any).headerPattern).toEqual( /^(\w*)(?:\((.*)\))?!?: (.*)$/ ); }); diff --git a/@commitlint/load/src/load.ts b/@commitlint/load/src/load.ts index 9f54ce20fe..277ad398f0 100644 --- a/@commitlint/load/src/load.ts +++ b/@commitlint/load/src/load.ts @@ -2,17 +2,14 @@ import executeRule from '@commitlint/execute-rule'; import resolveExtends from '@commitlint/resolve-extends'; import { LoadOptions, - ParserPreset, QualifiedConfig, QualifiedRules, + PluginRecords, UserConfig, - UserPreset, } from '@commitlint/types'; import isPlainObject from 'lodash/isPlainObject'; import merge from 'lodash/merge'; -import mergeWith from 'lodash/mergeWith'; -import pick from 'lodash/pick'; -import union from 'lodash/union'; +import uniq from 'lodash/uniq'; import Path from 'path'; import resolveFrom from 'resolve-from'; import {loadConfig} from './utils/load-config'; @@ -20,9 +17,6 @@ import {loadParserOpts} from './utils/load-parser-opts'; import loadPlugin from './utils/load-plugin'; import {pickConfig} from './utils/pick-config'; -const w = (_: unknown, b: ArrayLike | null | undefined | false) => - Array.isArray(b) ? b : undefined; - export default async function load( seed: UserConfig = {}, options: LoadOptions = {} @@ -35,11 +29,16 @@ export default async function load( // Might amount to breaking changes, defer until 9.0.0 // Merge passed config with file based options - const config = pickConfig(merge({}, loaded ? loaded.config : null, seed)); - - const opts = merge( - {extends: [], rules: {}, formatter: '@commitlint/format'}, - pick(config, 'extends', 'plugins', 'ignores', 'defaultIgnores') + const config = pickConfig( + merge( + { + extends: [], + plugins: [], + rules: {}, + }, + loaded ? loaded.config : null, + seed + ) ); // Resolve parserPreset key @@ -54,59 +53,35 @@ export default async function load( } // Resolve extends key - const extended = resolveExtends(opts, { + const extended = resolveExtends(config, { prefix: 'commitlint-config', cwd: base, parserPreset: config.parserPreset, - }); - - const preset = pickConfig( - mergeWith(extended, config, w) - ) as unknown as UserPreset; - preset.plugins = {}; - - // TODO: check if this is still necessary with the new factory based conventional changelog parsers - // config.extends = Array.isArray(config.extends) ? config.extends : []; + }) as unknown as UserConfig; - // Resolve parser-opts from preset - if (typeof preset.parserPreset === 'object') { - preset.parserPreset.parserOpts = await loadParserOpts( - preset.parserPreset.name, - // TODO: fix the types for factory based conventional changelog parsers - preset.parserPreset as any - ); + if (!extended.formatter || typeof extended.formatter !== 'string') { + extended.formatter = '@commitlint/format'; } - // Resolve config-relative formatter module - if (typeof config.formatter === 'string') { - preset.formatter = - resolveFrom.silent(base, config.formatter) || config.formatter; - } - - // Read plugins from extends + let plugins: PluginRecords = {}; if (Array.isArray(extended.plugins)) { - config.plugins = union(config.plugins, extended.plugins || []); - } - - // resolve plugins - if (Array.isArray(config.plugins)) { - config.plugins.forEach((plugin) => { + uniq(extended.plugins || []).forEach((plugin) => { if (typeof plugin === 'string') { - loadPlugin(preset.plugins, plugin, process.env.DEBUG === 'true'); + plugins = loadPlugin(plugins, plugin, process.env.DEBUG === 'true'); } else { - preset.plugins.local = plugin; + plugins.local = plugin; } }); } - const rules = preset.rules ? preset.rules : {}; - const qualifiedRules = ( + const rules = ( await Promise.all( - Object.entries(rules || {}).map((entry) => executeRule(entry)) + Object.entries(extended.rules || {}).map((entry) => executeRule(entry)) ) ).reduce((registry, item) => { - const [key, value] = item as any; - (registry as any)[key] = value; + // type of `item` can be null, but Object.entries always returns key pair + const [key, value] = item!; + registry[key] = value; return registry; }, {}); @@ -118,17 +93,24 @@ export default async function load( : 'https://github.com/conventional-changelog/commitlint/#what-is-commitlint'; const prompt = - preset.prompt && isPlainObject(preset.prompt) ? preset.prompt : {}; + extended.prompt && isPlainObject(extended.prompt) ? extended.prompt : {}; return { - extends: preset.extends!, - formatter: preset.formatter!, - parserPreset: preset.parserPreset! as ParserPreset, - ignores: preset.ignores!, - defaultIgnores: preset.defaultIgnores!, - plugins: preset.plugins!, - rules: qualifiedRules, - helpUrl, + extends: Array.isArray(extended.extends) + ? extended.extends + : typeof extended.extends === 'string' + ? [extended.extends] + : [], + // Resolve config-relative formatter module + formatter: + resolveFrom.silent(base, extended.formatter) || extended.formatter, + // Resolve parser-opts from preset + parserPreset: await loadParserOpts(extended.parserPreset), + ignores: extended.ignores, + defaultIgnores: extended.defaultIgnores, + plugins: plugins, + rules: rules, + helpUrl: helpUrl, prompt, }; } diff --git a/@commitlint/load/src/utils/load-parser-opts.ts b/@commitlint/load/src/utils/load-parser-opts.ts index 86d5b5b700..9d8c2d9390 100644 --- a/@commitlint/load/src/utils/load-parser-opts.ts +++ b/@commitlint/load/src/utils/load-parser-opts.ts @@ -1,48 +1,71 @@ +import {ParserPreset} from '@commitlint/types'; + +function isObjectLike(obj: unknown): obj is Record { + return Boolean(obj) && typeof obj === 'object'; // typeof null === 'object' +} + +function isParserOptsFunction( + obj: T +): obj is T & { + parserOpts: (...args: any[]) => any; +} { + return typeof obj.parserOpts === 'function'; +} + export async function loadParserOpts( - parserName: string, - pendingParser: Promise -) { + pendingParser: string | ParserPreset | Promise | undefined +): Promise { + if (!pendingParser || typeof pendingParser !== 'object') { + return undefined; + } // 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; + // exit early, no opts to resolve + if (!parser.parserOpts) { + return parser; + } + + // Pull nested parserOpts, might happen if overwritten with a module in main config + if (typeof parser.parserOpts === 'object') { + // Await parser opts if applicable + parser.parserOpts = await parser.parserOpts; + if ( + isObjectLike(parser.parserOpts) && + isObjectLike(parser.parserOpts.parserOpts) + ) { + parser.parserOpts = parser.parserOpts.parserOpts; + } + return parser; } // Create parser opts from factory if ( - typeof parser === 'object' && - typeof parser.parserOpts === 'function' && - parserName.startsWith('conventional-changelog-') + isParserOptsFunction(parser) && + typeof parser.name === 'string' && + parser.name.startsWith('conventional-changelog-') ) { - return await new Promise((resolve) => { - const result = parser.parserOpts((_: never, opts: {parserOpts: any}) => { - resolve(opts.parserOpts); + return new Promise((resolve) => { + const result = parser.parserOpts((_: never, opts: any) => { + resolve({ + ...parser, + parserOpts: opts?.parserOpts, + }); }); // If result has data or a promise, the parser doesn't support factory-init // due to https://github.com/nodejs/promises-debugging/issues/16 it just quits, so let's use this fallback if (result) { Promise.resolve(result).then((opts) => { - resolve(opts.parserOpts); + resolve({ + ...parser, + parserOpts: opts?.parserOpts, + }); }); } + return; }); } - // 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; - } - - return parser.parserOpts; + return parser; } diff --git a/@commitlint/load/src/utils/pick-config.ts b/@commitlint/load/src/utils/pick-config.ts index b4dd6fd6ce..08eb40f69c 100644 --- a/@commitlint/load/src/utils/pick-config.ts +++ b/@commitlint/load/src/utils/pick-config.ts @@ -1,7 +1,6 @@ -import {UserConfig} from '@commitlint/types'; import pick from 'lodash/pick'; -export const pickConfig = (input: unknown): UserConfig => +export const pickConfig = (input: unknown): Record => pick( input, 'extends', diff --git a/@commitlint/resolve-extends/src/index.test.ts b/@commitlint/resolve-extends/src/index.test.ts index b386d001c4..e651bce889 100644 --- a/@commitlint/resolve-extends/src/index.test.ts +++ b/@commitlint/resolve-extends/src/index.test.ts @@ -321,11 +321,7 @@ test('should fall back to conventional-changelog-lint-config prefix', () => { const require = (id: string) => { switch (id) { case 'conventional-changelog-lint-config-extender-name': - return { - rules: { - fallback: true, - }, - }; + return {rules: {fallback: true}}; default: return {}; } @@ -348,3 +344,188 @@ test('should fall back to conventional-changelog-lint-config prefix', () => { }, }); }); + +test('plugins should be merged correctly', () => { + const input = {extends: ['extender-name'], zero: 'root'}; + + const require = (id: string) => { + switch (id) { + case 'extender-name': + return {extends: ['recursive-extender-name'], plugins: ['test']}; + case 'recursive-extender-name': + return { + extends: ['second-recursive-extender-name'], + plugins: ['test2'], + }; + case 'second-recursive-extender-name': + return {plugins: ['test3']}; + default: + return {}; + } + }; + + const ctx = {resolve: id, require: jest.fn(require)} as ResolveExtendsContext; + + const actual = resolveExtends(input, ctx); + + const expected = { + extends: ['extender-name'], + plugins: ['test3', 'test2', 'test'], + zero: 'root', + }; + + expect(actual).toEqual(expected); +}); + +test('rules should be merged correctly', () => { + const input = {extends: ['extender-name'], rules: {test1: ['base', '1']}}; + + const require = (id: string) => { + switch (id) { + case 'extender-name': + return { + extends: ['recursive-extender-name'], + rules: {test2: [id, '2']}, + }; + case 'recursive-extender-name': + return { + extends: ['second-recursive-extender-name'], + rules: {test1: [id, '3']}, + }; + case 'second-recursive-extender-name': + return {rules: {test2: [id, '4']}}; + default: + return {}; + } + }; + + const ctx = {resolve: id, require: jest.fn(require)} as ResolveExtendsContext; + + const actual = resolveExtends(input, ctx); + + const expected = { + extends: ['extender-name'], + rules: { + test1: ['base', '1'], + test2: ['extender-name', '2'], + }, + }; + + expect(actual).toEqual(expected); +}); + +// 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); +}); + +test('parserPreset should be merged correctly', () => { + const input = {extends: ['extender-name'], zero: 'root'}; + + const require = (id: string) => { + switch (id) { + case 'extender-name': + return { + extends: ['recursive-extender-name'], + parserPreset: { + parserOpts: { + 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: ['#', '!'], + referenceActions: null, + }, + }, + zero: 'root', + }; + + expect(actual).toEqual(expected); +}); + +test('should correctly merge nested configs', () => { + const input = {extends: ['extender-1']}; + + const require = (id: string) => { + switch (id) { + case 'extender-1': + return {extends: ['extender-3', 'extender-2']}; + case 'extender-2': + return {extends: ['extender-4']}; + case 'extender-3': + return {rules: {test: 3}}; + case 'extender-4': + return {extends: ['extender-5', 'extender-6'], rules: {test: 4}}; + case 'extender-5': + return {rules: {test: 5}}; + case 'extender-6': + return {rules: {test: 6}}; + default: + return {}; + } + }; + + const ctx = {resolve: id, require: jest.fn(require)} as ResolveExtendsContext; + + const actual = resolveExtends(input, ctx); + + const expected = { + extends: ['extender-1'], + rules: { + test: 4, + }, + }; + + expect(actual).toEqual(expected); +}); diff --git a/@commitlint/resolve-extends/src/index.ts b/@commitlint/resolve-extends/src/index.ts index 505316f057..00a716bc55 100644 --- a/@commitlint/resolve-extends/src/index.ts +++ b/@commitlint/resolve-extends/src/index.ts @@ -13,7 +13,6 @@ export interface ResolvedConfig { } export interface ResolveExtendsConfig { - parserPreset?: unknown; extends?: string | string[]; helpUrl?: string; [key: string]: unknown; @@ -31,19 +30,23 @@ export interface ResolveExtendsContext { export default function resolveExtends( config: ResolveExtendsConfig = {}, context: ResolveExtendsContext = {} -) { +): ResolvedConfig { const {extends: e} = config; - const extended = loadExtends(config, context).reduce( + const extended = loadExtends(config, context); + extended.push(config); + return extended.reduce( (r, {extends: _, ...c}) => - mergeWith(r, c, (objValue, srcValue) => { - if (Array.isArray(objValue)) { + mergeWith(r, c, (objValue, srcValue, key) => { + if (key === 'plugins') { + if (Array.isArray(objValue)) { + return objValue.concat(srcValue); + } + } else if (Array.isArray(objValue)) { return srcValue; } }), e ? {extends: e} : {} ); - - return merge({}, extended, config); } function loadExtends( diff --git a/@commitlint/types/src/load.ts b/@commitlint/types/src/load.ts index aafd5c40d9..71099689c9 100644 --- a/@commitlint/types/src/load.ts +++ b/@commitlint/types/src/load.ts @@ -21,10 +21,10 @@ export interface LoadOptions { } export interface UserConfig { - extends?: string[]; + extends?: string | string[]; formatter?: string; rules?: Partial; - parserPreset?: string | ParserPreset; + parserPreset?: string | ParserPreset | Promise; ignores?: ((commit: string) => boolean)[]; defaultIgnores?: boolean; plugins?: (string | Plugin)[]; @@ -49,16 +49,16 @@ export interface QualifiedConfig { extends: string[]; formatter: string; rules: QualifiedRules; - parserPreset: ParserPreset; - ignores: ((commit: string) => boolean)[]; - defaultIgnores: boolean; + parserPreset?: ParserPreset; + ignores?: ((commit: string) => boolean)[]; + defaultIgnores?: boolean; plugins: PluginRecords; helpUrl: string; prompt: UserPromptConfig; } export interface ParserPreset { - name: string; - path: string; + name?: string; + path?: string; parserOpts?: unknown; }