Skip to content

Commit 90bb470

Browse files
committed
fix several bugs in handler closures - schema generation now works correctly except one remaining edge case; and, method calls are no longer being incorrectly captured
1 parent 26f6b59 commit 90bb470

17 files changed

+432
-57
lines changed

packages/html/src/jsx.d.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,8 +1763,8 @@ declare namespace CTDOM {
17631763

17641764
export interface HTMLAttributes<T> extends AriaAttributes, DOMAttributes<T> {
17651765
// CT extensions
1766-
"onClick"?: CellLike<HandlerEvent<unknown>>;
1767-
"onChange"?: CellLike<HandlerEvent<unknown>>;
1766+
"onClick"?: EventHandler<unknown>;
1767+
"onChange"?: EventHandler<unknown>;
17681768
"children"?: RenderNode | undefined;
17691769
// Allow React-isms
17701770
"key"?: number;
@@ -2825,10 +2825,12 @@ interface CTThemeDef {
28252825

28262826
type CTThemeInput = Partial<CTThemeDef> & Record<string, unknown>;
28272827

2828-
type HandlerEvent<T> = {
2828+
type CTEvent<T> = {
28292829
detail: T;
28302830
};
28312831

2832+
type EventHandler<T> = CellLike<CTEvent<T>> | ((event?: CTEvent<T>) => void);
2833+
28322834
// `Charm` is not a recipe type.
28332835
type Charm = any;
28342836

@@ -2889,16 +2891,16 @@ interface CTPlaidLinkElement extends CTHTMLElement {}
28892891

28902892
interface CTDraggableAttributes<T> extends CTHTMLAttributes<T> {
28912893
"key"?: number;
2892-
"x"?: CellLike<HandlerEvent<any>>;
2893-
"y"?: CellLike<HandlerEvent<any>>;
2894+
"x"?: CellLike<CTEvent<any>>;
2895+
"y"?: CellLike<CTEvent<any>>;
28942896
"hidden"?: Booleanish;
2895-
"onpositionchange"?: CellLike<HandlerEvent<any>>;
2897+
"onpositionchange"?: EventHandler<any>;
28962898
}
28972899

28982900
interface CTCanvasAttributes<T> extends CTHTMLAttributes<T> {
28992901
"width"?: string | number;
29002902
"height"?: string | number;
2901-
"onct-canvas-click"?: CellLike<HandlerEvent<any>>;
2903+
"onct-canvas-click"?: EventHandler<any>;
29022904
}
29032905

29042906
interface CTPlaidLinkAttributes<T> extends CTHTMLAttributes<T> {
@@ -2944,7 +2946,7 @@ interface CTAttachmentsBarAttributes<T> extends CTHTMLAttributes<T> {
29442946

29452947
interface CTTagsAttributes<T> extends CTHTMLAttributes<T> {
29462948
"tags"?: string[];
2947-
"onct-change"?: CellLike<HandlerEvent<any>>;
2949+
"onct-change"?: EventHandler<any>;
29482950
}
29492951

29502952
interface CTToolbarAttributes<T> extends CTHTMLAttributes<T> {
@@ -3017,7 +3019,7 @@ interface CTSendMessageAttributes<T> extends CTHTMLAttributes<T> {
30173019
"value"?: any;
30183020
"placeholder"?: string;
30193021
"appearance"?: "rounded";
3020-
"onmessagesend"?: CellLike<HandlerEvent<{ message: string }>>;
3022+
"onmessagesend"?: EventHandler<{ message: string }>;
30213023
"inline"?: Booleanish;
30223024
}
30233025

@@ -3031,7 +3033,7 @@ interface CTScrollAttributes<T> extends CTHTMLAttributes<T> {
30313033
interface CTOutlinerAttributes<T> extends CTHTMLAttributes<T> {
30323034
"$value": CellLike<{ root: OutlinerNode }>;
30333035
"$mentionable"?: CellLike<Charm[]>;
3034-
"oncharm-link-click"?: CellLike<HandlerEvent<{ charm: Cell<Charm> }>>;
3036+
"oncharm-link-click"?: EventHandler<{ charm: Cell<Charm> }>;
30353037
}
30363038

30373039
interface CTChatMessageAttributes<T> extends CTHTMLAttributes<T> {
@@ -3075,7 +3077,7 @@ interface CTListAttributes<T> extends CTHTMLAttributes<T> {
30753077
/** setting this hides the 'add item' form built into the list */
30763078
"readonly"?: boolean;
30773079
"title"?: string;
3078-
"onct-remove-item"?: CellLike<HandlerEvent<{ item: CtListItem }>>;
3080+
"onct-remove-item"?: EventHandler<{ item: CtListItem }>;
30793081
}
30803082

30813083
interface CTListItemAttributes<T> extends CTHTMLAttributes<T> {
@@ -3150,17 +3152,15 @@ interface CTCheckboxAttributes<T> extends CTHTMLAttributes<T> {
31503152
"indeterminate"?: boolean;
31513153
"name"?: string;
31523154
"value"?: string;
3153-
"onct-change"?: CellLike<HandlerEvent<any>>;
3155+
"onct-change"?: EventHandler<any>;
31543156
}
31553157

31563158
interface CTSelectAttributes<T> extends CTHTMLAttributes<T> {
31573159
"$value": CellLike<any | any[]>;
31583160
"items": { label: string; value: any }[];
31593161
"multiple"?: boolean;
3160-
"onct-change"?: CellLike<
3161-
HandlerEvent<
3162-
{ items: { label: string; value: any }[]; value: any | any[] }
3163-
>
3162+
"onct-change"?: EventHandler<
3163+
{ items: { label: string; value: any }[]; value: any | any[] }
31643164
>;
31653165
}
31663166

packages/schema-generator/src/schema-generator.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,9 @@ export class SchemaGenerator implements ISchemaGenerator {
612612
return { type: "boolean" };
613613
case ts.SyntaxKind.NullKeyword:
614614
return { type: "null" };
615+
case ts.SyntaxKind.NeverKeyword:
616+
// Reject all values (never type can never occur)
617+
return false as SchemaDefinition;
615618
case ts.SyntaxKind.UndefinedKeyword:
616619
case ts.SyntaxKind.VoidKeyword:
617620
case ts.SyntaxKind.AnyKeyword:

packages/ts-transformers/src/ast/dataflow.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
getExpressionText,
55
getMemberSymbol,
66
isFunctionParameter,
7+
isMethodCall,
78
} from "./utils.ts";
89
import { symbolDeclaresCommonToolsDefault } from "../core/mod.ts";
910
import { isOpaqueRefType } from "../transformers/opaque-ref/opaque-ref.ts";
@@ -428,11 +429,7 @@ export function createDataFlowAnalyzer(
428429

429430
// Don't capture property accesses that are method calls.
430431
// For example, `element.trim` in `element.trim()` should not be captured.
431-
if (
432-
expression.parent &&
433-
ts.isCallExpression(expression.parent) &&
434-
expression.parent.expression === expression
435-
) {
432+
if (isMethodCall(expression)) {
436433
// This is a method call like element.trim() - don't capture it
437434
return merged;
438435
}

packages/ts-transformers/src/ast/mod.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ export {
88
export {
99
getExpressionText,
1010
getMemberSymbol,
11+
getMethodCallTarget,
1112
isFunctionParameter,
13+
isMethodCall,
1214
visitEachChildWithJsx,
1315
} from "./utils.ts";
1416
export {

packages/ts-transformers/src/ast/type-inference.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,42 @@ export function typeToSchemaTypeNode(
175175
const result = typeToTypeNode(type, checker, location);
176176
return result;
177177
}
178+
179+
/**
180+
* If a parameter has an explicit type annotation that's not Any,
181+
* return it and register in TypeRegistry.
182+
* This is useful for transformers that need to preserve explicit types.
183+
*/
184+
export function tryExplicitParameterType(
185+
param: ts.ParameterDeclaration | undefined,
186+
checker: ts.TypeChecker,
187+
typeRegistry?: WeakMap<ts.Node, ts.Type>,
188+
): { typeNode: ts.TypeNode; type: ts.Type } | null {
189+
if (!param?.type) return null;
190+
191+
const annotationType = checker.getTypeFromTypeNode(param.type);
192+
193+
if (annotationType.flags & ts.TypeFlags.Any) return null;
194+
195+
if (typeRegistry) {
196+
typeRegistry.set(param.type, annotationType);
197+
}
198+
199+
return { typeNode: param.type, type: annotationType };
200+
}
201+
202+
/**
203+
* Create a TypeNode and register it with a Type in TypeRegistry.
204+
* Handles the common pattern of synthetic TypeNode creation.
205+
* This ensures that later transformer stages can retrieve the Type from synthetic nodes.
206+
*/
207+
export function registerTypeForNode(
208+
typeNode: ts.TypeNode,
209+
type: ts.Type,
210+
typeRegistry?: WeakMap<ts.Node, ts.Type>,
211+
): ts.TypeNode {
212+
if (typeRegistry) {
213+
typeRegistry.set(typeNode, type);
214+
}
215+
return typeNode;
216+
}

packages/ts-transformers/src/ast/utils.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,46 @@ export function visitEachChildWithJsx(
228228
// For all other nodes, use the default behavior
229229
return ts.visitEachChild(node, visitor, context);
230230
}
231+
232+
/**
233+
* Check if a property access expression is being invoked as a method call.
234+
*
235+
* @example
236+
* ```typescript
237+
* // Returns true:
238+
* obj.method() // node is obj.method
239+
*
240+
* // Returns false:
241+
* const x = obj.method // node is obj.method (not being called)
242+
* ```
243+
*/
244+
export function isMethodCall(node: ts.PropertyAccessExpression): boolean {
245+
return !!(
246+
node.parent &&
247+
ts.isCallExpression(node.parent) &&
248+
node.parent.expression === node
249+
);
250+
}
251+
252+
/**
253+
* When a property access is a method call, get the object being called on.
254+
* This is useful for closures that should capture the object, not the method.
255+
*
256+
* @example
257+
* ```typescript
258+
* state.counter.set() // Returns PropertyAccessExpression for state.counter
259+
* obj.method() // Returns undefined (obj is not a PropertyAccessExpression)
260+
* obj.prop // Returns undefined (not a method call)
261+
* ```
262+
*
263+
* @returns The object PropertyAccessExpression if this is a method call on a property chain,
264+
* undefined otherwise
265+
*/
266+
export function getMethodCallTarget(
267+
node: ts.PropertyAccessExpression,
268+
): ts.PropertyAccessExpression | undefined {
269+
if (!isMethodCall(node)) return undefined;
270+
271+
const obj = node.expression;
272+
return ts.isPropertyAccessExpression(obj) ? obj : undefined;
273+
}

0 commit comments

Comments
 (0)