Skip to content

Commit e9ef76c

Browse files
authored
fix(resolve-extends): always resolve extended parser presets for proper merging (#4647)
* fix(resolve-extends): always resolve extended parser presets for proper merging When a user provides a partial parserPreset (e.g. only issuePrefixes), the extended config's string parser preset was skipped because loadExtends checked !context.parserPreset before resolving. This meant the extended preset's headerPattern was never loaded, so it got lost during the config merge. Two changes fix this: 1. Remove the !context.parserPreset guard in loadExtends so string parser presets from extended configs are always resolved, making their parserOpts available for merging. 2. Preserve user-provided properties during the nested parserOpts unwrapping in loadParserOpts. Previously the unwrap replaced the outer object entirely, dropping any user additions that were merged at that level during config resolution. Fixes #4640 * fix: address review feedback from qodo and copilot - Shallow-copy the dynamicImport result to avoid mutating an immutable ESM namespace object when assigning the resolved parserPreset. - Add explicit type cast in load-parser-opts destructuring so inner is Record<string, unknown> instead of unknown. - Clarify unit test scope: it covers the merge path while the integration test covers string-to-object resolution.
1 parent 3b124a7 commit e9ef76c

File tree

7 files changed

+102
-11
lines changed

7 files changed

+102
-11
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = {
2+
extends: ['./extended'],
3+
parserPreset: {
4+
parserOpts: {
5+
issuePrefixes: ['PROJ-'],
6+
},
7+
},
8+
};
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module.exports = {
2+
parserOpts: {
3+
headerPattern: /^(\w*)(?:\((.*)\))?!?: (.*)$/,
4+
headerCorrespondence: ['type', 'scope', 'subject'],
5+
},
6+
};
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = {
2+
parserPreset: './conventional-changelog-custom',
3+
};

@commitlint/load/src/load.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,21 @@ test("parser preset overwrites completely instead of merging", async () => {
382382
});
383383
});
384384

385+
// https://github.com/conventional-changelog/commitlint/issues/4640
386+
test("partial user parserPreset merges with extended string parserPreset", async () => {
387+
const cwd = await gitBootstrap(
388+
"fixtures/parser-preset-partial-user-override",
389+
);
390+
const actual = await load({}, { cwd });
391+
392+
expect(actual.parserPreset).toBeDefined();
393+
expect(actual.parserPreset!.parserOpts).toMatchObject({
394+
headerPattern: /^(\w*)(?:\((.*)\))?!?: (.*)$/,
395+
headerCorrespondence: ["type", "scope", "subject"],
396+
issuePrefixes: ["PROJ-"],
397+
});
398+
});
399+
385400
test("recursive extends with parserPreset", async () => {
386401
const cwd = await gitBootstrap("fixtures/recursive-parser-preset");
387402
const actual = await load({}, { cwd });

@commitlint/load/src/utils/load-parser-opts.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,14 @@ export async function loadParserOpts(
4646
isObjectLike(parser.parserOpts) &&
4747
isObjectLike(parser.parserOpts.parserOpts)
4848
) {
49-
parser.parserOpts = parser.parserOpts.parserOpts;
49+
// Preserve any user-provided properties (e.g. issuePrefixes) that
50+
// were merged at the outer parserOpts level during config resolution,
51+
// while unwrapping the inner module-provided parserOpts (#4640).
52+
const { parserOpts: inner, ...rest } = parser.parserOpts as Record<
53+
string,
54+
unknown
55+
> & { parserOpts: Record<string, unknown> };
56+
parser.parserOpts = { ...inner, ...rest };
5057
}
5158
return parser;
5259
}

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,55 @@ test("parserPreset should be merged correctly", async () => {
540540
expect(actual).toEqual(expected);
541541
});
542542

543+
// https://github.com/conventional-changelog/commitlint/issues/4640
544+
// Verifies that mergeWith deep-merges parserPreset objects so that a
545+
// user's partial override (issuePrefixes) coexists with the extended
546+
// config's properties (headerPattern, headerCorrespondence).
547+
// The full string-to-object resolution path is covered by the
548+
// integration test in @commitlint/load ("partial user parserPreset
549+
// merges with extended string parserPreset").
550+
test("user partial parserPreset should merge with extended parserPreset", async () => {
551+
const input = {
552+
extends: ["extender-name"],
553+
parserPreset: {
554+
parserOpts: {
555+
issuePrefixes: ["PROJ-"],
556+
},
557+
},
558+
};
559+
560+
const dynamicImport = (id: string) => {
561+
switch (id) {
562+
case "extender-name":
563+
return {
564+
parserPreset: {
565+
parserOpts: {
566+
headerPattern: /^(\w*)(?:\((.*)\))?!?: (.*)$/,
567+
headerCorrespondence: ["type", "scope", "subject"],
568+
},
569+
},
570+
};
571+
default:
572+
return {};
573+
}
574+
};
575+
576+
const ctx = {
577+
resolve: id,
578+
dynamicImport: vi.fn(dynamicImport),
579+
} as ResolveExtendsContext;
580+
581+
const actual = await resolveExtends(input, ctx);
582+
583+
expect(actual.parserPreset).toEqual({
584+
parserOpts: {
585+
headerPattern: /^(\w*)(?:\((.*)\))?!?: (.*)$/,
586+
headerCorrespondence: ["type", "scope", "subject"],
587+
issuePrefixes: ["PROJ-"],
588+
},
589+
});
590+
});
591+
543592
test("should correctly merge nested configs", async () => {
544593
const input = { extends: ["extender-1"] };
545594

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,18 +139,21 @@ async function loadExtends(
139139
return await ext.reduce(async (configs, raw) => {
140140
const resolved = resolveConfig(raw, context);
141141

142-
const c = await (context.dynamicImport || dynamicImport)<{
143-
parserPreset?: string;
144-
}>(resolved);
142+
// Shallow-copy so we never mutate an ESM namespace object (#4647).
143+
const c = {
144+
...(await (context.dynamicImport || dynamicImport)<{
145+
parserPreset?: string | ParserPreset;
146+
}>(resolved)),
147+
};
145148
const cwd = path.dirname(resolved);
146149
const ctx = { ...context, cwd };
147150

148-
// Resolve parser preset if none was present before
149-
if (
150-
!context.parserPreset &&
151-
typeof c === "object" &&
152-
typeof c.parserPreset === "string"
153-
) {
151+
// Always resolve string parser presets from extended configs so that
152+
// their parserOpts (headerPattern, etc.) are available for merging.
153+
// Previously this was skipped when the user provided any parserPreset,
154+
// which caused partial user overrides (e.g. just issuePrefixes) to
155+
// lose the extended preset's headerPattern (see #4640).
156+
if (typeof c === "object" && typeof c.parserPreset === "string") {
154157
const resolvedParserPreset = resolveFrom(c.parserPreset, cwd);
155158

156159
const parserPreset: ParserPreset = {
@@ -159,7 +162,7 @@ async function loadExtends(
159162
};
160163

161164
ctx.parserPreset = parserPreset;
162-
config.parserPreset = parserPreset;
165+
c.parserPreset = parserPreset;
163166
}
164167

165168
validateConfig(resolved, config);

0 commit comments

Comments
 (0)