Skip to content

Commit a029ea0

Browse files
kraenhansenclaude
andcommitted
[DX-1319] Fix two false positives surfaced by the packages matrix
Discovered when the workspace run flagged @elevenlabs/client (which changes no source) as breaking: - Protected-member nominal artifact: TS phrases the protected variant of the cross-build nominal mismatch as "Property 'x' is protected but type 'Y' is not a class derived from 'Y'", which the private-only filter missed. Broaden isNominalAccessArtifact to catch any private/protected member nominal message. - Type-only re-export of a value: `export type { X }` where X resolves to a class is not in the value namespace, but getAliasedSymbol resolves through the `type` modifier and marked it a value export, so per-symbol localization indexed a name absent from `typeof import()` and raised a spurious TS2339. Honor the export-specifier's type-only modifier when classifying exports. Regression fixture nominal-reexport reproduces both; the real @elevenlabs/client surfaces (`.`, `./internal`, `./internal/unity`) now self-compare clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 42c5c99 commit a029ea0

6 files changed

Lines changed: 79 additions & 20 deletions

File tree

packages/dts-breaking-changes/src/analyze.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,23 @@ test("private member reached only through a parameter yields no false positive a
8181
);
8282
});
8383

84+
test("protected member via a parameter and a type-only re-export of a value produce no false positives", () => {
85+
// Across two builds: Conn.q (protected, reached via a parameter) trips the
86+
// whole-module gate as a nominal artifact, and `export type { Impl as ImplType }`
87+
// is a type-only re-export of a value that must not be localized as a value
88+
// export (which would raise a spurious "Property does not exist").
89+
const report = analyze({
90+
oldDir: fx("nominal-reexport/old"),
91+
newDir: fx("nominal-reexport/new"),
92+
config: { entry: "index.d.ts", gateDirection: "both" },
93+
});
94+
assert.deepEqual(
95+
report.findings,
96+
[],
97+
`expected zero findings, got: ${JSON.stringify(report.findings, null, 2)}`
98+
);
99+
});
100+
84101
test("dropping a method overload is a consumer-breaking change", () => {
85102
const report = analyze({
86103
oldDir: fx("overload/old"),

packages/dts-breaking-changes/src/analyze.ts

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,21 @@ function cleanMessage(message: string): string {
104104
}
105105

106106
/**
107-
* A "separate declarations of a private/protected property" (or "is private/
108-
* protected in type") diagnostic is a nominal artifact of comparing two
109-
* independent builds: access-restricted members are not part of the
110-
* consumer-visible contract, so such a mismatch can never be a real breaking
111-
* change. `MethodsToProperties` strips these where it recurses, but classes
112-
* reached only through parameter positions (kept positional to preserve variance
113-
* and overloads) slip through — hence this guard.
107+
* Diagnostics whose root cause is an access-restricted (private/protected)
108+
* member are nominal artifacts of comparing two independent builds: such members
109+
* are not part of the consumer-visible contract, so the mismatch can never be a
110+
* real breaking change. `MethodsToProperties` strips them where it recurses, but
111+
* classes reached only through parameter positions (kept positional to preserve
112+
* variance and overloads) slip through — hence this guard. TS phrases these
113+
* several ways ("separate declarations of a private property", "is protected but
114+
* type X is not a class derived from Y", "is private and only accessible ...").
115+
* A real access-narrowing change instead shows up as a *missing* member, so it
116+
* is not caught here.
114117
*/
115118
function isNominalAccessArtifact(message: string): boolean {
116-
return /separate declarations of a (private|protected) property|is (private|protected) in type/.test(
117-
message
119+
return (
120+
/separate declarations of a (private|protected) property/.test(message) ||
121+
/Property '[^']*' is (private|protected)\b/.test(message)
118122
);
119123
}
120124

@@ -176,9 +180,27 @@ function assertNoDeepInstantiation(
176180
type ExportKind = "value" | "type";
177181

178182
/**
179-
* Export names of a given kind. `value` = anything with value meaning
183+
* A `export type { X }` / `export { type X }` re-export never contributes to the
184+
* value namespace, even when X resolves to a value (e.g. a class) at its source —
185+
* so `getAliasedSymbol` alone can't be trusted. Honor the type-only modifier
186+
* before falling back to the resolved symbol's flags.
187+
*/
188+
function isValueExport(checker: ts.TypeChecker, sym: ts.Symbol): boolean {
189+
for (const decl of sym.getDeclarations() ?? []) {
190+
if (!ts.isExportSpecifier(decl)) continue;
191+
if (decl.isTypeOnly) return false;
192+
const clause = decl.parent.parent; // NamedExports -> ExportDeclaration
193+
if (ts.isExportDeclaration(clause) && clause.isTypeOnly) return false;
194+
}
195+
const resolved =
196+
sym.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(sym) : sym;
197+
return (resolved.flags & ts.SymbolFlags.Value) !== 0;
198+
}
199+
200+
/**
201+
* Export names of a given kind. `value` = anything in the value namespace
180202
* (class/function/const/enum/namespace); `type` = pure type-only exports
181-
* (interface/type alias) with no value meaning.
203+
* (interface/type alias, or a `export type` re-export).
182204
*/
183205
function exportNames(
184206
program: ts.Program,
@@ -190,17 +212,13 @@ function exportNames(
190212
if (!source) return [];
191213
const moduleSymbol = checker.getSymbolAtLocation(source);
192214
if (!moduleSymbol) return [];
193-
const TYPE_ONLY = ts.SymbolFlags.Interface | ts.SymbolFlags.TypeAlias;
194215
return checker
195216
.getExportsOfModule(moduleSymbol)
196-
.filter(sym => {
197-
const resolved =
198-
sym.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(sym) : sym;
199-
const isValue = (resolved.flags & ts.SymbolFlags.Value) !== 0;
200-
return kind === "value"
201-
? isValue
202-
: !isValue && (resolved.flags & TYPE_ONLY) !== 0;
203-
})
217+
.filter(sym =>
218+
kind === "value"
219+
? isValueExport(checker, sym)
220+
: !isValueExport(checker, sym)
221+
)
204222
.map(s => s.getName());
205223
}
206224

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export declare class Impl {
2+
x: number;
3+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export declare class Conn {
2+
protected q: number;
3+
}
4+
export interface Opts {
5+
conn: Conn;
6+
}
7+
export declare function configure(options: Opts): void;
8+
export { Impl } from "./impl.js";
9+
export type { Impl as ImplType } from "./impl.js";
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export declare class Impl {
2+
x: number;
3+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export declare class Conn {
2+
protected q: number;
3+
}
4+
export interface Opts {
5+
conn: Conn;
6+
}
7+
export declare function configure(options: Opts): void;
8+
export { Impl } from "./impl.js";
9+
export type { Impl as ImplType } from "./impl.js";

0 commit comments

Comments
 (0)