Skip to content

Commit 2808f39

Browse files
mrlubosmrlubos
authored
Merge pull request #3753 from inas-sirhan/feat/warn-duplicate-plugins
feat: warn when duplicate plugins are specified
2 parents e88f146 + 403b442 commit 2808f39

File tree

6 files changed

+278
-2
lines changed

6 files changed

+278
-2
lines changed

‎.changeset/warn-duplicate-plugins.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@hey-api/openapi-ts": patch
3+
"@hey-api/shared": patch
4+
---
5+
6+
**config**: warn on duplicated plugin configurations

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import type { AnyPluginName, PluginContext, PluginNames } from '@hey-api/shared';
2-
import { dependencyFactory, valueToObject } from '@hey-api/shared';
2+
import {
3+
dependencyFactory,
4+
valueToObject,
5+
warnOnConflictingDuplicatePlugins,
6+
} from '@hey-api/shared';
37

48
import { defaultPluginConfigs } from '../plugins/config';
59
import type { Config, UserConfig } from './types';
@@ -143,6 +147,8 @@ export function getPlugins({
143147
}
144148
}
145149

150+
warnOnConflictingDuplicatePlugins(definedPlugins);
151+
146152
const userPlugins = definedPlugins
147153
.map((plugin) => {
148154
if (typeof plugin === 'string') {

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import type { AnyPluginName, PluginContext, PluginNames } from '@hey-api/shared';
2-
import { dependencyFactory, valueToObject } from '@hey-api/shared';
2+
import {
3+
dependencyFactory,
4+
valueToObject,
5+
warnOnConflictingDuplicatePlugins,
6+
} from '@hey-api/shared';
37

48
import { defaultPluginConfigs } from '../plugins/config';
59
import type { Config, UserConfig } from './types';
@@ -146,6 +150,8 @@ export function getPlugins({
146150
}
147151
}
148152

153+
warnOnConflictingDuplicatePlugins(definedPlugins);
154+
149155
const userPlugins = definedPlugins
150156
.map((plugin) => {
151157
if (typeof plugin === 'string') {

‎packages/shared/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export type {
9999
OpenApiSchemaObject,
100100
} from './openApi/types';
101101
export type { GetNameContext, Hooks } from './parser/hooks';
102+
export { warnOnConflictingDuplicatePlugins } from './plugins/duplicate';
102103
export type { SchemaWithType } from './plugins/shared/types/schema';
103104
export { definePluginConfig, mappers } from './plugins/shared/utils/config';
104105
export type { PluginInstanceTypes } from './plugins/shared/utils/instance';

‎packages/shared/src/plugins/__tests__/duplicate.test.ts‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
import { log } from '@hey-api/codegen-core';
2+
3+
import { warnOnConflictingDuplicatePlugins } from '../duplicate';
4+
5+
describe('warnOnConflictingDuplicatePlugins', () => {
6+
afterEach(() => {
7+
vi.restoreAllMocks();
8+
});
9+
10+
const warningMessage =
11+
'Plugin "@hey-api/client-fetch" is configured multiple times. Only the last instance will take effect.';
12+
13+
it('does not warn for duplicate string plugins', () => {
14+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
15+
16+
warnOnConflictingDuplicatePlugins(['@hey-api/client-fetch', '@hey-api/client-fetch']);
17+
18+
expect(warnSpy).not.toHaveBeenCalled();
19+
});
20+
21+
it('does not warn for duplicate plugins with identical config in different key order', () => {
22+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
23+
24+
warnOnConflictingDuplicatePlugins([
25+
{
26+
foo: 'bar',
27+
name: '@hey-api/client-fetch',
28+
output: 'sdk',
29+
},
30+
{
31+
output: 'sdk',
32+
// eslint-disable-next-line sort-keys-fix/sort-keys-fix
33+
foo: 'bar',
34+
name: '@hey-api/client-fetch',
35+
},
36+
]);
37+
38+
expect(warnSpy).not.toHaveBeenCalled();
39+
});
40+
41+
it('does not warn when a string and an object with only name are specified', () => {
42+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
43+
44+
warnOnConflictingDuplicatePlugins(['@hey-api/client-fetch', { name: '@hey-api/client-fetch' }]);
45+
46+
expect(warnSpy).not.toHaveBeenCalled();
47+
});
48+
49+
it('does not warn for duplicate plugins with identical object config', () => {
50+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
51+
52+
warnOnConflictingDuplicatePlugins([
53+
{
54+
foo: 'bar',
55+
name: '@hey-api/client-fetch',
56+
},
57+
{
58+
foo: 'bar',
59+
name: '@hey-api/client-fetch',
60+
},
61+
]);
62+
63+
expect(warnSpy).not.toHaveBeenCalled();
64+
});
65+
66+
it('does not warn when nested object configs differ only in key order', () => {
67+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
68+
69+
warnOnConflictingDuplicatePlugins([
70+
{
71+
definitions: { case: 'PascalCase', name: 'foo' },
72+
name: '@hey-api/client-fetch',
73+
},
74+
{
75+
// eslint-disable-next-line sort-keys-fix/sort-keys-fix
76+
definitions: { name: 'foo', case: 'PascalCase' },
77+
name: '@hey-api/client-fetch',
78+
},
79+
]);
80+
81+
expect(warnSpy).not.toHaveBeenCalled();
82+
});
83+
84+
it('warns for duplicate plugins with conflicting config', () => {
85+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
86+
87+
warnOnConflictingDuplicatePlugins([
88+
{
89+
foo: 'bar',
90+
name: '@hey-api/client-fetch',
91+
},
92+
{
93+
foo: 'baz',
94+
name: '@hey-api/client-fetch',
95+
},
96+
]);
97+
98+
expect(warnSpy).toHaveBeenCalledOnce();
99+
expect(warnSpy).toHaveBeenCalledWith(warningMessage);
100+
});
101+
102+
it('warns when a string plugin conflicts with an object plugin of the same name', () => {
103+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
104+
105+
warnOnConflictingDuplicatePlugins([
106+
'@hey-api/client-fetch',
107+
{
108+
name: '@hey-api/client-fetch',
109+
output: 'sdk',
110+
},
111+
]);
112+
113+
expect(warnSpy).toHaveBeenCalledOnce();
114+
});
115+
116+
it('does not warn when array-valued options differ only in element key order', () => {
117+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
118+
119+
warnOnConflictingDuplicatePlugins([
120+
{
121+
items: [{ from: 'foo', name: 'bar' }],
122+
name: '@hey-api/client-fetch',
123+
},
124+
{
125+
// eslint-disable-next-line sort-keys-fix/sort-keys-fix
126+
items: [{ name: 'bar', from: 'foo' }],
127+
name: '@hey-api/client-fetch',
128+
},
129+
]);
130+
131+
expect(warnSpy).not.toHaveBeenCalled();
132+
});
133+
134+
it('warns when array-valued options differ in element order', () => {
135+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
136+
137+
warnOnConflictingDuplicatePlugins([
138+
{
139+
items: [
140+
{ from: 'a', name: 'x' },
141+
{ from: 'b', name: 'y' },
142+
],
143+
name: '@hey-api/client-fetch',
144+
},
145+
{
146+
items: [
147+
{ from: 'b', name: 'y' },
148+
{ from: 'a', name: 'x' },
149+
],
150+
name: '@hey-api/client-fetch',
151+
},
152+
]);
153+
154+
expect(warnSpy).toHaveBeenCalledOnce();
155+
expect(warnSpy).toHaveBeenCalledWith(warningMessage);
156+
});
157+
158+
it('does not warn when function-valued options have identical source', () => {
159+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
160+
const transform = (value: string) => value.toUpperCase();
161+
162+
warnOnConflictingDuplicatePlugins([
163+
{
164+
definitions: { name: transform },
165+
name: '@hey-api/client-fetch',
166+
},
167+
{
168+
definitions: { name: transform },
169+
name: '@hey-api/client-fetch',
170+
},
171+
]);
172+
173+
expect(warnSpy).not.toHaveBeenCalled();
174+
});
175+
176+
it('warns when function-valued options differ', () => {
177+
const warnSpy = vi.spyOn(log, 'warn').mockImplementation(() => {});
178+
179+
warnOnConflictingDuplicatePlugins([
180+
{
181+
definitions: { name: (value: string) => value.toUpperCase() },
182+
name: '@hey-api/client-fetch',
183+
},
184+
{
185+
definitions: { name: (value: string) => value.toLowerCase() },
186+
name: '@hey-api/client-fetch',
187+
},
188+
]);
189+
190+
expect(warnSpy).toHaveBeenCalledOnce();
191+
expect(warnSpy).toHaveBeenCalledWith(warningMessage);
192+
});
193+
});

‎packages/shared/src/plugins/duplicate.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { log } from '@hey-api/codegen-core';
2+
3+
import type { PluginNames } from './types';
4+
5+
type PluginConfig = {
6+
name: PluginNames;
7+
};
8+
9+
type PluginDefinition<TConfig extends PluginConfig = PluginConfig> = PluginNames | TConfig;
10+
11+
function stableStringify(value: unknown): string {
12+
return JSON.stringify(value, (_, v) => {
13+
if (typeof v === 'function') {
14+
return `[function:${(v as () => unknown).toString()}]`;
15+
}
16+
if (v && typeof v === 'object' && !Array.isArray(v)) {
17+
return Object.fromEntries(
18+
Object.entries(v as Record<string, unknown>).sort(([a], [b]) => a.localeCompare(b)),
19+
);
20+
}
21+
return v;
22+
});
23+
}
24+
25+
function normalizePluginEntry<TConfig extends PluginConfig>(
26+
plugin: PluginDefinition<TConfig>,
27+
): {
28+
name: PluginNames;
29+
serialized: string;
30+
} {
31+
if (typeof plugin === 'string') {
32+
return {
33+
name: plugin,
34+
serialized: '{}',
35+
};
36+
}
37+
38+
const { name, ...config } = plugin;
39+
40+
return {
41+
name,
42+
serialized: stableStringify(config),
43+
};
44+
}
45+
46+
export function warnOnConflictingDuplicatePlugins<TConfig extends PluginConfig>(
47+
plugins: ReadonlyArray<PluginDefinition<TConfig>>,
48+
): void {
49+
const seen = new Map<string, string>();
50+
51+
for (const plugin of plugins) {
52+
const { name, serialized } = normalizePluginEntry(plugin);
53+
if (!name) continue;
54+
55+
const previous = seen.get(name);
56+
if (previous !== undefined && previous !== serialized) {
57+
log.warn(
58+
`Plugin "${name}" is configured multiple times. Only the last instance will take effect.`,
59+
);
60+
}
61+
62+
seen.set(name, serialized);
63+
}
64+
}

0 commit comments

Comments
 (0)