Skip to content

Commit 839a59e

Browse files
mathpiratemathpirate
committed
Avoid recomputing computed destructured aliases and add fixture
1 parent 013947b commit 839a59e

File tree

6 files changed

+159
-20
lines changed

6 files changed

+159
-20
lines changed

‎packages/ts-transformers/src/closures/transformer.ts‎

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -897,13 +897,53 @@ function transformDestructuredProperties(
897897
): ts.ConciseBody {
898898
const elemName = elemParam?.name;
899899

900-
// Collect destructured property names if the param is an object destructuring pattern
900+
let transformedBody: ts.ConciseBody = body;
901+
902+
const prependStatements = (
903+
statements: readonly ts.Statement[],
904+
currentBody: ts.ConciseBody,
905+
): ts.ConciseBody => {
906+
if (statements.length === 0) return currentBody;
907+
908+
if (ts.isBlock(currentBody)) {
909+
return factory.updateBlock(
910+
currentBody,
911+
factory.createNodeArray([
912+
...statements,
913+
...currentBody.statements,
914+
]),
915+
);
916+
}
917+
918+
return factory.createBlock(
919+
[
920+
...statements,
921+
factory.createReturnStatement(currentBody as ts.Expression),
922+
],
923+
true,
924+
);
925+
};
926+
901927
const destructuredProps = new Map<string, () => ts.Expression>();
928+
const computedInitializers: ts.VariableStatement[] = [];
929+
const usedTempNames = new Set<string>();
930+
931+
const registerTempName = (base: string): string => {
932+
let candidate = `__ct_${base || "prop"}_key`;
933+
let counter = 1;
934+
while (usedTempNames.has(candidate)) {
935+
candidate = `__ct_${base || "prop"}_key_${counter++}`;
936+
}
937+
usedTempNames.add(candidate);
938+
return candidate;
939+
};
940+
902941
if (elemName && ts.isObjectBindingPattern(elemName)) {
903942
for (const element of elemName.elements) {
904943
if (ts.isBindingElement(element) && ts.isIdentifier(element.name)) {
905944
const alias = element.name.text;
906945
const propertyName = element.propertyName;
946+
usedTempNames.add(alias);
907947

908948
destructuredProps.set(alias, () => {
909949
const target = factory.createIdentifier("element");
@@ -930,9 +970,29 @@ function transformDestructuredProperties(
930970
}
931971

932972
if (ts.isComputedPropertyName(propertyName)) {
973+
const tempName = registerTempName(alias);
974+
const tempIdentifier = factory.createIdentifier(tempName);
975+
976+
computedInitializers.push(
977+
factory.createVariableStatement(
978+
undefined,
979+
factory.createVariableDeclarationList(
980+
[
981+
factory.createVariableDeclaration(
982+
tempIdentifier,
983+
undefined,
984+
undefined,
985+
propertyName.expression,
986+
),
987+
],
988+
ts.NodeFlags.Const,
989+
),
990+
),
991+
);
992+
933993
return factory.createElementAccessExpression(
934994
target,
935-
propertyName.expression,
995+
tempIdentifier,
936996
);
937997
}
938998

@@ -945,7 +1005,6 @@ function transformDestructuredProperties(
9451005
}
9461006
}
9471007

948-
// Collect array destructured identifiers: [date, pizza] -> {date: 0, pizza: 1}
9491008
const arrayDestructuredVars = new Map<string, number>();
9501009
if (elemName && ts.isArrayBindingPattern(elemName)) {
9511010
let index = 0;
@@ -957,12 +1016,9 @@ function transformDestructuredProperties(
9571016
}
9581017
}
9591018

960-
// If param was object-destructured, replace property references with element.prop
9611019
if (destructuredProps.size > 0) {
9621020
const visitor: ts.Visitor = (node) => {
9631021
if (ts.isIdentifier(node) && destructuredProps.has(node.text)) {
964-
// Check if this identifier is not part of a property access already
965-
// (e.g., don't transform the 'x' in 'something.x')
9661022
if (
9671023
!node.parent ||
9681024
!(ts.isPropertyAccessExpression(node.parent) &&
@@ -974,16 +1030,14 @@ function transformDestructuredProperties(
9741030
}
9751031
return visitEachChildWithJsx(node, visitor, undefined);
9761032
};
977-
return ts.visitNode(body, visitor) as ts.ConciseBody;
1033+
transformedBody = ts.visitNode(transformedBody, visitor) as ts.ConciseBody;
9781034
}
9791035

980-
// If param was array-destructured, replace variable references with element[index]
9811036
if (arrayDestructuredVars.size > 0) {
9821037
const visitor: ts.Visitor = (node) => {
9831038
if (ts.isIdentifier(node)) {
9841039
const index = arrayDestructuredVars.get(node.text);
9851040
if (index !== undefined) {
986-
// Check if this identifier is not part of a property access already
9871041
if (
9881042
!node.parent ||
9891043
!(ts.isPropertyAccessExpression(node.parent) &&
@@ -998,10 +1052,14 @@ function transformDestructuredProperties(
9981052
}
9991053
return visitEachChildWithJsx(node, visitor, undefined);
10001054
};
1001-
return ts.visitNode(body, visitor) as ts.ConciseBody;
1055+
transformedBody = ts.visitNode(transformedBody, visitor) as ts.ConciseBody;
10021056
}
10031057

1004-
return body;
1058+
if (computedInitializers.length > 0) {
1059+
transformedBody = prependStatements(computedInitializers, transformedBody);
1060+
}
1061+
1062+
return transformedBody;
10051063
}
10061064

10071065
/**

‎packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.expected.tsx‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import * as __ctHelpers from "commontools";
2+
import { recipe, UI } from "commontools";
3+
let keyCounter = 0;
4+
function nextKey() {
5+
return `value-${keyCounter++}`;
6+
}
7+
interface State {
8+
items: Array<Record<string, number>>;
9+
}
10+
export default recipe({
11+
type: "object",
12+
properties: {
13+
items: {
14+
type: "array",
15+
items: {
16+
type: "object",
17+
properties: {},
18+
additionalProperties: {
19+
type: "number"
20+
}
21+
}
22+
}
23+
},
24+
required: ["items"]
25+
} as const satisfies __ctHelpers.JSONSchema, (state) => {
26+
return {
27+
[UI]: (<div>
28+
{state.items.mapWithPattern(__ctHelpers.recipe({
29+
$schema: "https://json-schema.org/draft/2020-12/schema",
30+
type: "object",
31+
properties: {
32+
element: {
33+
type: "object",
34+
properties: {},
35+
additionalProperties: {
36+
type: "number"
37+
}
38+
},
39+
params: {
40+
type: "object",
41+
properties: {}
42+
}
43+
},
44+
required: ["element", "params"]
45+
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => {
46+
const __ct_amount_key = nextKey();
47+
return (<span>{__ctHelpers.derive({ element, __ct_amount_key }, ({ element: element, __ct_amount_key: __ct_amount_key }) => element[__ct_amount_key])}</span>);
48+
}), {})}
49+
</div>),
50+
};
51+
});
52+
// @ts-ignore: Internals
53+
function h(...args: any[]) { return __ctHelpers.h.apply(null, args); }
54+
// @ts-ignore: Internals
55+
h.fragment = __ctHelpers.h.fragment;

‎packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.input.tsx‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <cts-enable />
2+
import { recipe, UI } from "commontools";
3+
4+
let keyCounter = 0;
5+
function nextKey() {
6+
return `value-${keyCounter++}`;
7+
}
8+
9+
interface State {
10+
items: Array<Record<string, number>>;
11+
}
12+
13+
export default recipe<State>("ComputedAliasSideEffect", (state) => {
14+
return {
15+
[UI]: (
16+
<div>
17+
{state.items.map(({ [nextKey()]: amount }) => (
18+
<span>{amount}</span>
19+
))}
20+
</div>
21+
),
22+
};
23+
});

‎packages/ts-transformers/test/fixtures/closures/map-destructured-computed-alias.expected.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@ export default recipe({
6464
required: ["value", "other"]
6565
}
6666
}
67-
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (<span>{__ctHelpers.derive(element, element => element[dynamicKey])}</span>)), {})}
67+
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => {
68+
const __ct_val_key = dynamicKey;
69+
return (<span>{__ctHelpers.derive({ element, __ct_val_key }, ({ element: element, __ct_val_key: __ct_val_key }) => element[__ct_val_key])}</span>);
70+
}), {})}
6871
</div>),
6972
};
7073
});

‎packages/ts-transformers/test/fixtures/closures/map-destructured-numeric-alias.expected.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as __ctHelpers from "commontools";
22
import { recipe, UI } from "commontools";
33
interface State {
44
entries: Array<{
5-
zero: number;
5+
0: number;
66
}>;
77
}
88
export default recipe({
@@ -13,11 +13,11 @@ export default recipe({
1313
items: {
1414
type: "object",
1515
properties: {
16-
zero: {
16+
0: {
1717
type: "number"
1818
}
1919
},
20-
required: ["zero"]
20+
required: ["0"]
2121
}
2222
}
2323
},
@@ -32,19 +32,19 @@ export default recipe({
3232
element: {
3333
type: "object",
3434
properties: {
35-
zero: {
35+
0: {
3636
type: "number"
3737
}
3838
},
39-
required: ["zero"]
39+
required: ["0"]
4040
},
4141
params: {
4242
type: "object",
4343
properties: {}
4444
}
4545
},
4646
required: ["element", "params"]
47-
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (<span>{element.zero}</span>)), {})}
47+
} as const satisfies __ctHelpers.JSONSchema, ({ element, params: {} }) => (<span>{element[0]}</span>)), {})}
4848
</div>),
4949
};
5050
});

‎packages/ts-transformers/test/fixtures/closures/map-destructured-numeric-alias.input.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
import { recipe, UI } from "commontools";
33

44
interface State {
5-
entries: Array<{ zero: number }>;
5+
entries: Array<{ 0: number }>;
66
}
77

88
export default recipe<State>("MapDestructuredNumericAlias", (state) => {
99
return {
1010
[UI]: (
1111
<div>
12-
{state.entries.map(({ zero: first }) => (
12+
{state.entries.map(({ 0: first }) => (
1313
<span>{first}</span>
1414
))}
1515
</div>

0 commit comments

Comments
 (0)