Skip to content

Commit adcfc47

Browse files
committed
feat: only warn on duplicate plugins with conflicting options
1 parent f8d9acd commit adcfc47

File tree

2 files changed

+146
-29
lines changed

2 files changed

+146
-29
lines changed

packages/openapi-ts/src/__tests__/index.test.ts

Lines changed: 118 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -286,33 +286,133 @@ describe('createClient', () => {
286286
expect(results).toHaveLength(4);
287287
});
288288

289-
it('warns when duplicate plugins are specified', async () => {
290-
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
291-
292-
await createClient({
293-
dryRun: true,
289+
describe('duplicate plugin warnings', () => {
290+
const baseConfig = {
291+
dryRun: true as const,
294292
input: {
295293
info: { title: 'duplicate-plugin-test', version: '1.0.0' },
296-
openapi: '3.1.0',
294+
openapi: '3.1.0' as const,
297295
},
298296
logs: {
299-
level: 'silent',
297+
level: 'silent' as const,
300298
},
301299
output: 'output',
302-
plugins: [
303-
{ name: '@hey-api/typescript' },
304-
{ name: '@hey-api/typescript' },
305-
],
300+
};
301+
302+
const conflictWarnings = (warnSpy: ReturnType<typeof vi.spyOn>) =>
303+
warnSpy.mock.calls.filter(
304+
(args: unknown[]) => typeof args[0] === 'string' && args[0].includes('conflicting options'),
305+
);
306+
307+
it('warns when the same plugin is specified with conflicting options', async () => {
308+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
309+
310+
await createClient({
311+
...baseConfig,
312+
plugins: [
313+
{ case: 'PascalCase', name: '@hey-api/typescript' },
314+
{ case: 'camelCase', name: '@hey-api/typescript' },
315+
],
316+
});
317+
318+
expect(conflictWarnings(warnSpy)).toHaveLength(1);
319+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('"@hey-api/typescript"'));
320+
321+
warnSpy.mockRestore();
322+
});
323+
324+
it('does not warn when the same plugin is specified twice as a string', async () => {
325+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
326+
327+
await createClient({
328+
...baseConfig,
329+
plugins: ['@hey-api/typescript', '@hey-api/typescript'],
330+
});
331+
332+
expect(conflictWarnings(warnSpy)).toHaveLength(0);
333+
334+
warnSpy.mockRestore();
335+
});
336+
337+
it('does not warn when a string and an object with only name are specified', async () => {
338+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
339+
340+
await createClient({
341+
...baseConfig,
342+
plugins: ['@hey-api/typescript', { name: '@hey-api/typescript' }],
343+
});
344+
345+
expect(conflictWarnings(warnSpy)).toHaveLength(0);
346+
347+
warnSpy.mockRestore();
348+
});
349+
350+
it('does not warn when two identical object configurations are specified', async () => {
351+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
352+
353+
await createClient({
354+
...baseConfig,
355+
plugins: [
356+
{ case: 'PascalCase', name: '@hey-api/typescript' },
357+
{ case: 'PascalCase', name: '@hey-api/typescript' },
358+
],
359+
});
360+
361+
expect(conflictWarnings(warnSpy)).toHaveLength(0);
362+
363+
warnSpy.mockRestore();
364+
});
365+
366+
it('does not warn when objects differ only in key order', async () => {
367+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
368+
369+
await createClient({
370+
...baseConfig,
371+
plugins: [
372+
{ case: 'PascalCase', name: '@hey-api/typescript' },
373+
{ name: '@hey-api/typescript', case: 'PascalCase' },
374+
],
375+
});
376+
377+
expect(conflictWarnings(warnSpy)).toHaveLength(0);
378+
379+
warnSpy.mockRestore();
380+
});
381+
382+
it('does not warn when nested object configs differ only in key order', async () => {
383+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
384+
385+
await createClient({
386+
...baseConfig,
387+
plugins: [
388+
{
389+
definitions: { case: 'PascalCase', name: 'foo' },
390+
name: '@hey-api/typescript',
391+
},
392+
{
393+
name: '@hey-api/typescript',
394+
definitions: { name: 'foo', case: 'PascalCase' },
395+
},
396+
],
397+
});
398+
399+
expect(conflictWarnings(warnSpy)).toHaveLength(0);
400+
401+
warnSpy.mockRestore();
306402
});
307403

308-
expect(warnSpy).toHaveBeenCalledWith(
309-
expect.stringContaining('Duplicate plugin'),
310-
);
311-
expect(warnSpy).toHaveBeenCalledWith(
312-
expect.stringContaining('"@hey-api/typescript"'),
313-
);
404+
it('warns when an object adds extra config compared to a string entry', async () => {
405+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
314406

315-
warnSpy.mockRestore();
407+
await createClient({
408+
...baseConfig,
409+
plugins: ['@hey-api/typescript', { case: 'PascalCase', name: '@hey-api/typescript' }],
410+
});
411+
412+
expect(conflictWarnings(warnSpy)).toHaveLength(1);
413+
414+
warnSpy.mockRestore();
415+
});
316416
});
317417

318418
it('executes @angular/common HttpRequest builder path', async () => {

packages/openapi-ts/src/config/plugins.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,30 +147,47 @@ export function getPlugins({
147147
}
148148
}
149149

150-
const seenPlugins = new Set<string>();
150+
const seenPlugins = new Map<string, string>();
151+
152+
const stableStringify = (value: unknown): string =>
153+
JSON.stringify(value, (_, v) =>
154+
v && typeof v === 'object' && !Array.isArray(v)
155+
? Object.fromEntries(
156+
Object.entries(v as Record<string, unknown>).sort(([a], [b]) => a.localeCompare(b)),
157+
)
158+
: v,
159+
);
151160

152-
const warnDuplicatePlugin = (name: string) =>
161+
const warnConflictingPlugin = (name: string) =>
153162
console.warn(
154-
`⚙️ ${colors.yellow('Warning:')} Duplicate plugin ${colors.cyan(`"${name}"`)} detected. Only the last occurrence will take effect.`,
163+
`⚙️ ${colors.yellow('Warning:')} Plugin ${colors.cyan(`"${name}"`)} is configured more than once with conflicting options. Only the last occurrence will take effect.`,
155164
);
156165

166+
const checkDuplicate = (name: string, config: Record<string, unknown>) => {
167+
const serialized = stableStringify(config);
168+
const previous = seenPlugins.get(name);
169+
if (previous !== undefined && previous !== serialized) {
170+
warnConflictingPlugin(name);
171+
}
172+
seenPlugins.set(name, serialized);
173+
};
174+
157175
const userPlugins = definedPlugins
158176
.map((plugin) => {
159177
if (typeof plugin === 'string') {
160-
if (seenPlugins.has(plugin)) {
161-
warnDuplicatePlugin(plugin);
162-
}
163-
seenPlugins.add(plugin);
178+
checkDuplicate(plugin, {});
164179
return plugin;
165180
}
166181

167182
const pluginName = plugin.name;
168183

169184
if (pluginName) {
170-
if (seenPlugins.has(pluginName)) {
171-
warnDuplicatePlugin(pluginName);
172-
}
173-
seenPlugins.add(pluginName);
185+
const config = Object.fromEntries(
186+
Object.entries(plugin as unknown as Record<string, unknown>).filter(
187+
([key]) => key !== 'name',
188+
),
189+
);
190+
checkDuplicate(pluginName, config);
174191

175192
// @ts-expect-error
176193
if (plugin.handler) {

0 commit comments

Comments
 (0)