also looking at this
fix(extractor): remove the implementation signature, not an overload
#3120
2 files
+95
-10
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -126,7 +126,9 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 126 | 126 | const len = source.length |
| 127 | 127 | const declarations: Declaration[] = [] |
| 128 | 128 | const nonExportedTypes = new Map<string, Declaration>() |
| 129 | const funcBodyIndices = new Set<number>() | |
| 129 | // Functions read with a body: the implementation signature of an overload set. | |
| 130 | // Held by identity, since declarations are inserted before them later. | |
| 131 | const funcBodies = new Set<Declaration>() | |
| 130 | 132 | let pos = 0 |
| 131 | 133 | |
| 132 | 134 | // Skip BOM (byte order mark) |
| @@ -2211,12 +2213,7 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 2211 | 2213 | const text = `${exportPrefix}${declarationName}${generics}${dtsParams}: ${returnType};` |
| 2212 | 2214 | const comments = extractLeadingComments(declStart) |
| 2213 | 2215 | |
| 2214 | // Record index if this function had a body (implementation signature for overloads) | |
| 2215 | if (hasBody) { | |
| 2216 | funcBodyIndices.add(declarations.length) | |
| 2217 | } | |
| 2218 | ||
| 2219 | return { | |
| 2216 | const decl: Declaration = { | |
| 2220 | 2217 | kind: 'function', |
| 2221 | 2218 | name: name || 'default', |
| 2222 | 2219 | text, |
| @@ -2230,6 +2227,9 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 2230 | 2227 | start: declStart, |
| 2231 | 2228 | end: pos, |
| 2232 | 2229 | } |
| 2230 | if (hasBody) | |
| 2231 | funcBodies.add(decl) | |
| 2232 | return decl | |
| 2233 | 2233 | } |
| 2234 | 2234 | |
| 2235 | 2235 | /** |
| @@ -4057,11 +4057,16 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 4057 | 4057 | // Post-process: remove implementation signatures of overloaded functions. |
| 4058 | 4058 | // For any function name that appears more than once, remove the last declaration |
| 4059 | 4059 | // that has a body (the implementation signature). |
| 4060 | if (funcBodyIndices.size > 0) { | |
| 4060 | if (funcBodies.size > 0) { | |
| 4061 | // A default export and a named export of the same name are separate | |
| 4062 | // functions (`export function make(...)` beside `export default function | |
| 4063 | // make(...) {}`), so each is its own overload set. | |
| 4064 | const overloadKey = (decl: Declaration): string => decl.isDefault ? `default ${decl.name}` : decl.name | |
| 4061 | 4065 | const funcNameCounts = new Map<string, number>() |
| 4062 | 4066 | for (const decl of declarations) { |
| 4063 | 4067 | if (decl.kind === 'function') { |
| 4064 | funcNameCounts.set(decl.name, (funcNameCounts.get(decl.name) || 0) + 1) | |
| 4068 | const key = overloadKey(decl) | |
| 4069 | funcNameCounts.set(key, (funcNameCounts.get(key) || 0) + 1) | |
| 4065 | 4070 | } |
| 4066 | 4071 | } |
| 4067 | 4072 | // Only remove implementation signatures for overloaded functions (count > 1) |
| @@ -4076,7 +4081,7 @@ function scanDeclarationsInternal(_source: string, _filename: string, _keepComme | ||
| 4076 | 4081 | for (const name of overloadedNames) { |
| 4077 | 4082 | // Walk backwards to find the last declaration with a body for this name |
| 4078 | 4083 | for (let i = declarations.length - 1; i >= 0; i--) { |
| 4079 | if (declarations[i].kind === 'function' && declarations[i].name === name && funcBodyIndices.has(i)) { | |
| 4084 | if (declarations[i].kind === 'function' && overloadKey(declarations[i]) === name && funcBodies.has(declarations[i])) { | |
| 4080 | 4085 | toRemove.add(i) |
| 4081 | 4086 | break |
| 4082 | 4087 | } |
| @@ -0,0 +1,80 @@ | ||
| 1 | /** | |
| 2 | * An overloaded function lost a public overload and kept its implementation | |
| 3 | * signature when a local type was pulled into the declaration file: | |
| 4 | * | |
| 5 | * interface I { a: number } | |
| 6 | * export function f(x: string): string | |
| 7 | * export function f(x: number): number | |
| 8 | * export function f(x: unknown): unknown { return x } | |
| 9 | * export const value: I = { a: 1 } | |
| 10 | * | |
| 11 | * emitted `f(x: string)` and `f(x: unknown): unknown`, so `f(1)` became | |
| 12 | * `unknown` and `f(true)` type-checked. The implementation was remembered by | |
| 13 | * its index in `declarations`, and declaring `I` shifted every index after it, | |
| 14 | * so the removal hit the overload before the implementation. | |
| 15 | */ | |
| 16 | import { describe, expect, it } from 'bun:test' | |
| 17 | import { mkdtemp, rm, writeFile } from 'node:fs/promises' | |
| 18 | import { tmpdir } from 'node:os' | |
| 19 | import { join } from 'node:path' | |
| 20 | import { runTsc } from './helpers/tsc' | |
| 21 | import { processCode } from './test-utils' | |
| 22 | ||
| 23 | /** Type-check `consumer` against the generated declarations as `./index`. */ | |
| 24 | async function checkConsumer(dts: string, consumer: string): Promise<string> { | |
| 25 | const dir = await mkdtemp(join(tmpdir(), 'dtsx-overloads-')) | |
| 26 | try { | |
| 27 | await writeFile(join(dir, 'index.d.ts'), dts) | |
| 28 | await writeFile(join(dir, 'use.ts'), consumer) | |
| 29 | return runTsc(dir, ['index.d.ts', 'use.ts']).output | |
| 30 | } | |
| 31 | finally { | |
| 32 | await rm(dir, { recursive: true, force: true }) | |
| 33 | } | |
| 34 | } | |
| 35 | ||
| 36 | describe('overloads alongside a declared local type', () => { | |
| 37 | it.each([ | |
| 38 | [ | |
| 39 | 'local interface before the overloads', | |
| 40 | 'interface I { a: number }\nexport function f(x: string): string\nexport function f(x: number): number\nexport function f(x: unknown): unknown { return x }\nexport const value: I = { a: 1 }\n', | |
| 41 | ['f(x: string): string;', 'f(x: number): number;'], | |
| 42 | 'f(x: unknown)', | |
| 43 | 'import { f } from \'./index\'\nconst s: string = f(\'a\')\nconst n: number = f(1)\n// @ts-expect-error no public overload takes a boolean\nf(true)\nexport {}\n', | |
| 44 | ], | |
| 45 | [ | |
| 46 | 'default export', | |
| 47 | 'interface Opts { a: number }\nexport default function make(x: string): Opts\nexport default function make(x: number): number\nexport default function make(x: any): any { return x }\n', | |
| 48 | ['make(x: string): Opts;', 'make(x: number): number;'], | |
| 49 | 'make(x: any)', | |
| 50 | 'import make from \'./index\'\nconst o: { a: number } = make(\'a\')\nconst n: number = make(1)\n// @ts-expect-error no public overload takes a boolean\nmake(true)\nexport {}\n', | |
| 51 | ], | |
| 52 | [ | |
| 53 | 'two overloaded functions', | |
| 54 | 'interface A { a: number }\ninterface B { b: number }\nexport function f(x: string): A\nexport function f(x: number): number\nexport function f(x: any): any { return x }\nexport function g(x: string): B\nexport function g(x: number): number\nexport function g(x: any): any { return x }\n', | |
| 55 | ['f(x: string): A;', 'f(x: number): number;', 'g(x: string): B;', 'g(x: number): number;'], | |
| 56 | '(x: any)', | |
| 57 | 'import { f, g } from \'./index\'\nconst a: { a: number } = f(\'a\')\nconst b: { b: number } = g(\'b\')\n// @ts-expect-error no public overload takes a boolean\nf(true)\n// @ts-expect-error no public overload takes a boolean\ng(true)\nexport {}\n', | |
| 58 | ], | |
| 59 | ])('keeps the public overloads: %s', async (_label, code, overloads, implementation, consumer) => { | |
| 60 | const output = processCode(code) | |
| 61 | for (const overload of overloads) | |
| 62 | expect(output).toContain(overload) | |
| 63 | expect(output).not.toContain(implementation) | |
| 64 | expect(await checkConsumer(output, consumer)).toBe('') | |
| 65 | }) | |
| 66 | }) | |
| 67 | ||
| 68 | describe('a named and a default export of the same name', () => { | |
| 69 | it.each([ | |
| 70 | ['named signature, default implementation', 'interface Input { value: number }\ninterface Output { result: number }\nexport function make(input: Input): Output\nexport default function make(input: Input): Output { return { result: input.value } }\n'], | |
| 71 | ['default signature, named implementation', 'interface Input { value: number }\ninterface Output { result: number }\nexport default function make(input: Input): Output\nexport function make(input: Input): Output { return { result: input.value } }\n'], | |
| 72 | ['no local types', 'export function make(input: { value: number }): { result: number }\nexport default function make(input: { value: number }): { result: number } { return { result: input.value } }\n'], | |
| 73 | ])('keeps both: %s', async (_label, code) => { | |
| 74 | const output = processCode(code) | |
| 75 | expect(output).toContain('export declare function make(') | |
| 76 | expect(output).toContain('export default function make(') | |
| 77 | const consumer = 'import make, { make as named } from \'./index\'\nconst a: number = make({ value: 1 }).result\nconst b: number = named({ value: 1 }).result\nexport {}\n' | |
| 78 | expect(await checkConsumer(output, consumer)).toBe('') | |
| 79 | }) | |
| 80 | }) | |