Skip to content

Commit caabc9f

Browse files
mrlubosmrlubos
authored
Merge pull request #3291 from hey-api/copilot/fix-duplicate-import-error
2 parents 124dc28 + 3c08e38 commit caabc9f

3 files changed

Lines changed: 282 additions & 3 deletions

File tree

‎.changeset/selfish-seas-add.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hey-api/codegen-core": patch
3+
---
4+
5+
**planner**: fix duplicate import when same symbol is imported as both type and value

‎packages/codegen-core/src/__tests__/planner.test.ts‎

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
import type { INode } from '../nodes/node';
2+
import { Project } from '../project/project';
3+
import { ref } from '../refs/refs';
4+
import type { Symbol } from '../symbols/symbol';
5+
import type { SymbolKind } from '../symbols/types';
6+
7+
/**
8+
* Creates a mock node for testing.
9+
*/
10+
const createMockNode = (args: {
11+
dependencies?: Array<Symbol>;
12+
filePath: string;
13+
language?: 'typescript' | 'javascript';
14+
name: string;
15+
project: Project;
16+
symbolKind?: SymbolKind;
17+
}): { node: INode; symbol: Symbol } => {
18+
const {
19+
dependencies = [],
20+
filePath,
21+
language = 'typescript',
22+
name,
23+
project,
24+
symbolKind = 'var',
25+
} = args;
26+
27+
const symbol = project.symbols.register({
28+
exported: true,
29+
getFilePath: () => filePath,
30+
kind: symbolKind,
31+
name,
32+
});
33+
34+
const node: INode = {
35+
analyze: (ctx) => {
36+
for (const dep of dependencies) {
37+
ctx.addDependency(ref(dep));
38+
}
39+
},
40+
clone () {
41+
return this;
42+
},
43+
exported: true,
44+
language,
45+
name: ref(name) as INode['name'],
46+
scope: 'value',
47+
symbol,
48+
toAst: () => ({}),
49+
'~brand': 'test-node',
50+
};
51+
52+
symbol.setNode(node);
53+
project.nodes.add(node);
54+
55+
return { node, symbol };
56+
};
57+
58+
describe('Planner imports deduplication', () => {
59+
it('produces a single value import when there is 1 imported symbol as value', () => {
60+
const project = new Project({ root: '/root' });
61+
62+
// Create source file with exported symbol
63+
const { symbol: sourceSymbol } = createMockNode({
64+
filePath: 'source',
65+
name: 'MyValue',
66+
project,
67+
symbolKind: 'var',
68+
});
69+
70+
// Create consumer file that imports the source symbol as value
71+
createMockNode({
72+
dependencies: [sourceSymbol],
73+
filePath: 'consumer',
74+
name: 'Consumer',
75+
project,
76+
});
77+
78+
project.plan();
79+
80+
const consumerFile = [...project.files.registered()].find((f) => f.name === 'consumer');
81+
expect(consumerFile).toBeDefined();
82+
83+
const imports = consumerFile!.imports;
84+
expect(imports).toHaveLength(1);
85+
expect(imports[0]!.imports).toHaveLength(1);
86+
expect(imports[0]!.imports[0]!.isTypeOnly).toBe(false);
87+
expect(imports[0]!.imports[0]!.localName).toBe('MyValue');
88+
});
89+
90+
it('produces a single type import when there is 1 imported symbol as type', () => {
91+
const project = new Project({ root: '/root' });
92+
93+
// Create source file with exported type symbol
94+
const { symbol: sourceSymbol } = createMockNode({
95+
filePath: 'source',
96+
name: 'MyType',
97+
project,
98+
symbolKind: 'type',
99+
});
100+
101+
// Create consumer file that imports the source symbol as type
102+
createMockNode({
103+
dependencies: [sourceSymbol],
104+
filePath: 'consumer',
105+
name: 'Consumer',
106+
project,
107+
});
108+
109+
project.plan();
110+
111+
const consumerFile = [...project.files.registered()].find((f) => f.name === 'consumer');
112+
expect(consumerFile).toBeDefined();
113+
114+
const imports = consumerFile!.imports;
115+
expect(imports).toHaveLength(1);
116+
expect(imports[0]!.imports).toHaveLength(1);
117+
expect(imports[0]!.imports[0]!.isTypeOnly).toBe(true);
118+
expect(imports[0]!.imports[0]!.localName).toBe('MyType');
119+
});
120+
121+
it('produces a single value import when there are 2 imported symbols as values with same name', () => {
122+
const project = new Project({ root: '/root' });
123+
124+
// Create source file with exported symbol
125+
const { symbol: sourceSymbol } = createMockNode({
126+
filePath: 'source',
127+
name: 'MyValue',
128+
project,
129+
symbolKind: 'var',
130+
});
131+
132+
// Create consumer file that imports the source symbol twice as value
133+
const consumerSymbol = project.symbols.register({
134+
exported: true,
135+
getFilePath: () => 'consumer',
136+
kind: 'var',
137+
name: 'Consumer',
138+
});
139+
const consumerNode: INode = {
140+
analyze: (ctx) => {
141+
ctx.addDependency(ref(sourceSymbol));
142+
ctx.addDependency(ref(sourceSymbol));
143+
},
144+
clone () {
145+
return this;
146+
},
147+
exported: true,
148+
language: 'typescript',
149+
name: ref('Consumer') as INode['name'],
150+
scope: 'value',
151+
symbol: consumerSymbol,
152+
toAst: () => ({}),
153+
'~brand': 'test-node',
154+
};
155+
consumerSymbol.setNode(consumerNode);
156+
project.nodes.add(consumerNode);
157+
158+
project.plan();
159+
160+
const consumerFile = [...project.files.registered()].find((f) => f.name === 'consumer');
161+
expect(consumerFile).toBeDefined();
162+
163+
const imports = consumerFile!.imports;
164+
expect(imports).toHaveLength(1);
165+
expect(imports[0]!.imports).toHaveLength(1);
166+
expect(imports[0]!.imports[0]!.isTypeOnly).toBe(false);
167+
expect(imports[0]!.imports[0]!.localName).toBe('MyValue');
168+
});
169+
170+
it('produces a single type import when there are 2 imported symbols as types with same name', () => {
171+
const project = new Project({ root: '/root' });
172+
173+
// Create source file with exported type symbol
174+
const { symbol: sourceSymbol } = createMockNode({
175+
filePath: 'source',
176+
name: 'MyType',
177+
project,
178+
symbolKind: 'type',
179+
});
180+
181+
// Create consumer file that imports the source symbol twice as type
182+
const consumerSymbol = project.symbols.register({
183+
exported: true,
184+
getFilePath: () => 'consumer',
185+
kind: 'var',
186+
name: 'Consumer',
187+
});
188+
const consumerNode: INode = {
189+
analyze: (ctx) => {
190+
ctx.addDependency(ref(sourceSymbol));
191+
ctx.addDependency(ref(sourceSymbol));
192+
},
193+
clone () {
194+
return this;
195+
},
196+
exported: true,
197+
language: 'typescript',
198+
name: ref('Consumer') as INode['name'],
199+
scope: 'value',
200+
symbol: consumerSymbol,
201+
toAst: () => ({}),
202+
'~brand': 'test-node',
203+
};
204+
consumerSymbol.setNode(consumerNode);
205+
project.nodes.add(consumerNode);
206+
207+
project.plan();
208+
209+
const consumerFile = [...project.files.registered()].find((f) => f.name === 'consumer');
210+
expect(consumerFile).toBeDefined();
211+
212+
const imports = consumerFile!.imports;
213+
expect(imports).toHaveLength(1);
214+
expect(imports[0]!.imports).toHaveLength(1);
215+
expect(imports[0]!.imports[0]!.isTypeOnly).toBe(true);
216+
expect(imports[0]!.imports[0]!.localName).toBe('MyType');
217+
});
218+
219+
it('produces a single value import when there are 2 imported symbols, 1 as type and 1 as value, with same name', () => {
220+
const project = new Project({ root: '/root' });
221+
222+
// Create source file with exported type symbol
223+
const { symbol: typeSymbol } = createMockNode({
224+
filePath: 'source',
225+
name: 'MySymbol',
226+
project,
227+
symbolKind: 'type',
228+
});
229+
230+
// Create source file with exported value symbol with same name
231+
const { symbol: valueSymbol } = createMockNode({
232+
filePath: 'source',
233+
name: 'MySymbol',
234+
project,
235+
symbolKind: 'var',
236+
});
237+
238+
// Create consumer file that imports both symbols (type and value with same name)
239+
const consumerSymbol = project.symbols.register({
240+
exported: true,
241+
getFilePath: () => 'consumer',
242+
kind: 'var',
243+
name: 'Consumer',
244+
});
245+
const consumerNode: INode = {
246+
analyze: (ctx) => {
247+
ctx.addDependency(ref(typeSymbol));
248+
ctx.addDependency(ref(valueSymbol));
249+
},
250+
clone () {
251+
return this;
252+
},
253+
exported: true,
254+
language: 'typescript',
255+
name: ref('Consumer') as INode['name'],
256+
scope: 'value',
257+
symbol: consumerSymbol,
258+
toAst: () => ({}),
259+
'~brand': 'test-node',
260+
};
261+
consumerSymbol.setNode(consumerNode);
262+
project.nodes.add(consumerNode);
263+
264+
project.plan();
265+
266+
const consumerFile = [...project.files.registered()].find((f) => f.name === 'consumer');
267+
expect(consumerFile).toBeDefined();
268+
269+
const imports = consumerFile!.imports;
270+
expect(imports).toHaveLength(1);
271+
expect(imports[0]!.imports).toHaveLength(1);
272+
expect(imports[0]!.imports[0]!.isTypeOnly).toBe(false);
273+
expect(imports[0]!.imports[0]!.localName).toBe('MySymbol');
274+
});
275+
});

‎packages/codegen-core/src/planner/planner.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,8 @@ export class Planner {
266266

267267
const fromFileId = dep.file.id;
268268
const importedName = dep.finalName;
269-
const isTypeOnly = isTypeOnlyKind(dep.kind);
270269
const kind = dep.importKind;
271-
const key = `${fromFileId}|${importedName}|${kind}|${isTypeOnly}`;
270+
const key = `${fromFileId}|${importedName}|${kind}`;
272271

273272
let entry = fileMap.get(key);
274273
if (!entry) {
@@ -292,8 +291,8 @@ export class Planner {
292291
symbol: imp,
293292
};
294293
fileMap.set(key, entry);
295-
entry.kinds.add(imp.kind);
296294
}
295+
entry.kinds.add(dep.kind);
297296

298297
dependency['~ref'] = entry.symbol;
299298
});

0 commit comments

Comments
 (0)