Skip to content

Commit d120968

Browse files
committed
fix: deduplicate destructure props to prevent duplicate bindings
buildDestructurePatternProps now skips destructureProps entries that already exist in baseProps. This prevents duplicate bindings like `const { href, href, ...rest } = props` when pseudo guard props overlap with intrinsic props (e.g. href/target on links, type/readOnly on inputs). https://claude.ai/code/session_01CEoafpHoAaRMVmm1WtcXu1
1 parent 6e3615c commit d120968

File tree

2 files changed

+71
-1
lines changed

2 files changed

+71
-1
lines changed
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { describe, expect, it } from "vitest";
2+
import jscodeshift from "jscodeshift";
3+
import { buildDestructurePatternProps } from "../internal/emit-wrappers/jsx-builders.js";
4+
import { patternProp } from "../internal/transform-utils.js";
5+
6+
const j = jscodeshift.withParser("tsx");
7+
const pp = (name: string) => patternProp(j, name);
8+
9+
describe("buildDestructurePatternProps", () => {
10+
it("skips destructureProps that already exist in baseProps", () => {
11+
// Simulates the link emitter where baseProps already has "href" and "target"
12+
// and pseudoGuardProps also contains "href" from a guard condition.
13+
const baseProps = [pp("href"), pp("target"), pp("children")];
14+
15+
const result = buildDestructurePatternProps(j, pp, {
16+
baseProps,
17+
destructureProps: ["href", "newProp"],
18+
includeRest: true,
19+
restId: j.identifier("rest"),
20+
});
21+
22+
const names = result.map((p) => {
23+
if (p.type === "RestElement") {
24+
return "...rest";
25+
}
26+
const key = (p as { key?: { name?: string } }).key;
27+
return key?.name ?? "?";
28+
});
29+
30+
// "href" should NOT be duplicated; "newProp" should be added
31+
expect(names).toEqual(["href", "target", "children", "newProp", "...rest"]);
32+
});
33+
34+
it("skips destructureProps that match 'as' prop with AssignmentPattern value", () => {
35+
// The "as" prop in baseProps uses j.property.from() with an AssignmentPattern value
36+
const asProp = j.property.from({
37+
kind: "init",
38+
key: j.identifier("as"),
39+
value: j.assignmentPattern(j.identifier("Component"), j.literal("div")),
40+
shorthand: false,
41+
});
42+
const baseProps = [asProp, pp("className")];
43+
44+
const result = buildDestructurePatternProps(j, pp, {
45+
baseProps,
46+
destructureProps: ["as", "extraProp"],
47+
});
48+
49+
const names = result.map((p) => {
50+
const key = (p as { key?: { name?: string } }).key;
51+
return key?.name ?? "?";
52+
});
53+
54+
expect(names).toEqual(["as", "className", "extraProp"]);
55+
});
56+
});

src/internal/emit-wrappers/jsx-builders.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,21 @@ export function buildDestructurePatternProps(
313313
const { baseProps, destructureProps, propDefaults, includeRest = false, restId } = args;
314314
const patternProps: Array<Property | RestElement> = [...baseProps];
315315

316-
for (const name of destructureProps.filter((n): n is string => Boolean(n))) {
316+
// Collect names already present in baseProps to avoid duplicate bindings
317+
// (e.g. pseudo guard props overlapping with intrinsic props like href/type)
318+
const existingNames = new Set<string>();
319+
for (const prop of baseProps) {
320+
if (prop.type !== "RestElement") {
321+
const key = (prop as { key?: { type?: string; name?: string } }).key;
322+
if (key?.type === "Identifier" && key.name) {
323+
existingNames.add(key.name);
324+
}
325+
}
326+
}
327+
328+
for (const name of destructureProps.filter(
329+
(n): n is string => typeof n === "string" && n.length > 0 && !existingNames.has(n),
330+
)) {
317331
const defaultVal = propDefaults?.get(name);
318332
if (defaultVal !== undefined) {
319333
patternProps.push(buildShorthandDefaultPatternProp(j, name, defaultVal));

0 commit comments

Comments
 (0)