From 88331bb8cca5be680cb072240a7da719be51428b Mon Sep 17 00:00:00 2001 From: meh Date: Mon, 31 Aug 2026 14:57:48 +0700 Subject: [PATCH 1/4] fix(library): claim the props a component declares as its own The runtime already routes declared props away from the root element: `behaviour` names them, `routedKeys` excludes them from `passthrough`, and the root slot spreads only what is left. The compiler never populated it, so for every compiled component the list was empty and each of its own props was also spread onto the root. For most props that is invisible: a stray attribute on a wrapper. For a prop whose name is a DOM event it is not. A component declaring onInput?: (value: string) => void wires that to an inner input and calls it with the string. With the same prop on the wrapper too, the inner event bubbles up and calls the caller's handler a second time with the raw InputEvent. The bubbled call lands last, so that is the one the caller sees, and the signature it was written against is a lie. `onChange` is declared this way by eight layouts, and consumers had already started writing `typeof value !== "string"` guards without knowing why they were needed. Which props are the component's own is a question the props type answers. It is an intersection, and the two halves differ: a referenced member -- `UIBaseProps`, `JSX.ButtonHTMLAttributes` -- is inherited HTML that belongs on the element, while an inline object literal is what this component itself accepts and places. So the literal's keys, and only those, are emitted as `behaviour`. A props type that cannot be read locally, because it is imported, emits nothing and keeps the previous behaviour. That is the safe direction: it leaves a prop on the element rather than dropping it. Checked against a 98-component library: of 661 declared keys the only ones a layout does not itself place are recipe keys, which presentation claims first, and `children`, which the layout reads through `_stable`. No HTML attribute stops reaching an element. --- .../crates/transform/src/lib.rs | 187 +++++++++++++++++- packages/solid-layouts/src/component.test.ts | 26 +++ 2 files changed, 207 insertions(+), 6 deletions(-) diff --git a/packages/solid-layouts-oxc/crates/transform/src/lib.rs b/packages/solid-layouts-oxc/crates/transform/src/lib.rs index 65f7300..e7e59f8 100644 --- a/packages/solid-layouts-oxc/crates/transform/src/lib.rs +++ b/packages/solid-layouts-oxc/crates/transform/src/lib.rs @@ -22,8 +22,8 @@ use layouts_common::{ }; use oxc_allocator::Allocator; use oxc_ast::ast::{ - BindingPattern, Declaration, Expression, Program, Statement, TSType, TSTypeName, - VariableDeclaration, VariableDeclarator, + BindingPattern, Declaration, Expression, Program, Statement, TSSignature, TSType, + TSTypeAliasDeclaration, TSTypeName, VariableDeclaration, VariableDeclarator, }; use oxc_codegen::Codegen; use oxc_parser::Parser; @@ -45,6 +45,8 @@ pub struct FoundLayout { pub parameters_span: Option, pub body_span: Option, pub props_span: Option, + /// Name of the props type, when it is a plain reference we can look up. + pub props_type: Option, pub statement_span: Span, pub export_prefix_span: Option, } @@ -330,10 +332,28 @@ fn compile_library_source( } else { "" }; + // What the component declares is its own API, and the runtime has + // to be told so it does not spread those props onto the root + // element on top of wherever the layout puts them. + let declared = layout + .props_type + .as_deref() + .map(|name| declared_prop_keys(program, name)) + .unwrap_or_default(); + let behaviour = if declared.is_empty() { + String::new() + } else { + let keys = declared + .iter() + .map(|key| format!("\"{key}\"")) + .collect::>() + .join(", "); + format!(", behaviour: [{keys}]") + }; edits.push(SourceEdit::Insert { at: layout.statement_span.end as usize, text: format!( - "\n{exported}const {} = __defineLayoutComponent({{ recipe: {}, layout: {raw}, embedded: true }}) as __LayoutComponent<{props}>;", + "\n{exported}const {} = __defineLayoutComponent({{ recipe: {}, layout: {raw}, embedded: true{behaviour} }}) as __LayoutComponent<{props}>;", layout.binding, layout.recipe.as_deref().expect("validated Layout recipe"), ), @@ -453,6 +473,77 @@ fn variable_declaration<'a, 'b>( } } +/// The type alias behind a statement, whether or not it is exported. +fn type_alias_declaration<'a, 'b>( + statement: &'b Statement<'a>, +) -> Option<&'b TSTypeAliasDeclaration<'a>> { + match statement { + Statement::TSTypeAliasDeclaration(declaration) => Some(declaration), + Statement::ExportDeclaration(export) => match &export.declaration { + Declaration::TSTypeAliasDeclaration(declaration) => Some(declaration), + _ => None, + }, + _ => None, + } +} + +/// The prop names a component declares as its own API. +/// +/// A props type is an intersection, and the two halves mean different things. +/// A referenced member -- `UIBaseProps`, `JSX.ButtonHTMLAttributes` -- is +/// inherited HTML, and those props belong on the element. An inline object +/// literal is what this component itself accepts, and the layout is what +/// places them. +/// +/// Only the literal's keys are the component's own, and the runtime needs to +/// know them so it does not also spread them onto the root element. A +/// component declaring `onInput?: (value: string) => void` and wiring it to an +/// inner input had the caller's handler bound to the wrapper as well; the +/// inner event bubbled up to it and called the handler a second time with the +/// raw InputEvent, which is not what the signature promises. The same held for +/// every `onChange`, `onSubmit` and `onInput` that a layout translates. +fn declared_prop_keys(program: &Program<'_>, name: &str) -> Vec { + for statement in &program.body { + let Some(alias) = type_alias_declaration(statement) else { + continue; + }; + if alias.id.name.as_str() != name { + continue; + } + let mut keys = Vec::new(); + collect_literal_keys(&alias.type_annotation, &mut keys); + return keys; + } + Vec::new() +} + +/// Keys of every inline object literal in a props type, intersections included. +fn collect_literal_keys(annotation: &TSType<'_>, keys: &mut Vec) { + match annotation { + TSType::TSTypeLiteral(literal) => { + for member in &literal.members { + if let TSSignature::TSPropertySignature(property) = member + && let Some(key) = property.key.static_name() + { + let key = key.to_string(); + if !keys.contains(&key) { + keys.push(key); + } + } + } + } + TSType::TSIntersectionType(intersection) => { + for part in &intersection.types { + collect_literal_keys(part, keys); + } + } + TSType::TSParenthesizedType(parenthesized) => { + collect_literal_keys(&parenthesized.type_annotation, keys); + } + _ => {} + } +} + fn as_layout( declarator: &VariableDeclarator<'_>, statement_span: Span, @@ -487,11 +578,15 @@ fn as_layout( }, _ => None, }); - let props_span = reference + let props = reference .type_arguments .as_ref() - .and_then(|arguments| arguments.params.get(1)) - .map(GetSpan::span); + .and_then(|arguments| arguments.params.get(1)); + let props_span = props.map(GetSpan::span); + let props_type = props.and_then(|argument| match argument { + TSType::TSTypeReference(reference) => type_name(&reference.type_name), + _ => None, + }); let (parameters, parameters_span, body_span) = match declarator.init.as_ref() { Some(Expression::ArrowFunctionExpression(arrow)) => ( @@ -511,6 +606,7 @@ fn as_layout( parameters_span, body_span, props_span, + props_type, statement_span, export_prefix_span: exported .then(|| Span::new(statement_span.start, declaration_span.start)), @@ -750,6 +846,85 @@ const Button: Layout = () => { ); } + #[test] + fn component_output_routes_the_props_a_component_declares_itself() { + let source = r#"import type { Layout } from "solid-layouts"; +import { passwordField } from "./PasswordField.recipe"; +export type PasswordFieldProps = UIBaseProps & { + value?: string; + onInput?: (value: string) => void; + onBlur?: () => void; +}; +export const PasswordField: Layout = () => { + return props.onInput?.(event.currentTarget.value)} />; +}; +"#; + let mut options = TransformOptions::new("PasswordField.layout.tsx", CompilerMode::Library); + options.library_output = LibraryOutput::Component; + let result = transform(source, &options); + assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); + // Declared props are the component's own API. Without this the runtime + // also spreads them onto the root, and an inner input's event bubbles + // back to the caller's handler with the raw event. + assert!( + result + .code + .contains("embedded: true, behaviour: [\"value\", \"onInput\", \"onBlur\"] })"), + "{}", + result.code + ); + } + + #[test] + fn component_output_leaves_inherited_html_props_as_passthrough() { + let source = r#"import type { Layout } from "solid-layouts"; +import { button } from "./Button.recipe"; +export type ButtonProps = Omit, "type"> & + UIBaseProps & { + variant?: Variant; + }; +export const Button: Layout = () => { + return ; +}; +"#; + let mut options = TransformOptions::new("Button.layout.tsx", CompilerMode::Library); + options.library_output = LibraryOutput::Component; + let result = transform(source, &options); + assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); + // `onClick` and the rest of the HTML surface arrive through a referenced + // type, so they stay passthrough and still reach the element. + assert!( + result.code.contains("behaviour: [\"variant\"] })"), + "{}", + result.code + ); + assert!(!result.code.contains("onClick"), "{}", result.code); + } + + #[test] + fn component_output_omits_behaviour_when_the_props_type_is_not_local() { + let source = r#"import type { Layout } from "solid-layouts"; +import type { BadgeProps } from "./types"; +import { badge } from "./Badge.recipe"; +export const Badge: Layout = () => { + return {props.children}; +}; +"#; + let mut options = TransformOptions::new("Badge.layout.tsx", CompilerMode::Library); + options.library_output = LibraryOutput::Component; + let result = transform(source, &options); + assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); + // Nothing to read means nothing claimed: the previous behaviour, which + // is the safe direction to fail in. + assert!( + result + .code + .contains("embedded: true }) as __LayoutComponent;"), + "{}", + result.code + ); + } + #[test] fn component_output_wraps_the_generated_layout_as_a_solid_component() { let source = r#"import type { Layout } from "solid-layouts"; diff --git a/packages/solid-layouts/src/component.test.ts b/packages/solid-layouts/src/component.test.ts index ca7054a..dc911c8 100644 --- a/packages/solid-layouts/src/component.test.ts +++ b/packages/solid-layouts/src/component.test.ts @@ -101,6 +101,32 @@ describe("defineComponent: the props split", () => { dispose(); }); + test("a declared prop stays off the root slot", () => { + // The bug this covers: a component declaring `onInput?: (value: string) + // => void` wires it to an inner input itself. If the same prop is also + // spread onto the root, the inner event bubbles back up to the wrapper + // and calls the caller's handler a second time with the raw InputEvent, + // which is not what the signature promises. Whichever fires last wins, + // and that is the bubbled one. + const { seen, layout } = capturing(); + + const Field = defineComponent({ + recipe: button, + layout: layout as never, + behaviour: ["onInput"], + setup: () => ({ loading: () => false }), + }); + + const handler = () => {}; + const dispose = mount(Field, { onInput: handler, onClick: handler }); + + expect(seen.slot?.root).not.toHaveProperty("onInput"); + // The contrast that keeps this honest: an undeclared handler is plain + // HTML and still has to reach the element. + expect(seen.slot?.root?.onClick).toBe(handler); + dispose(); + }); + test("undeclared props are HTML and reach the element", () => { // The bucket that did not exist: `id`, `onClick`, `aria-label` and // `data-testid` were swallowed as behaviour and never rendered. From 429215b40a75bde66c0c3e45118dd65a57b56327 Mon Sep 17 00:00:00 2001 From: meh Date: Mon, 31 Aug 2026 15:30:43 +0700 Subject: [PATCH 2/4] chore(oxc): 0.2.3 --- packages/solid-layouts-oxc/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/solid-layouts-oxc/package.json b/packages/solid-layouts-oxc/package.json index 69bebeb..1c7a829 100644 --- a/packages/solid-layouts-oxc/package.json +++ b/packages/solid-layouts-oxc/package.json @@ -1,6 +1,6 @@ { "name": "solid-layouts-oxc", - "version": "0.2.2", + "version": "0.2.3", "description": "The Layouts pre-pass for SolidJS, built on oxc", "license": "MIT", "type": "commonjs", From 224761e85c85ecf8a8403d66ae9d8badd3987ef2 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 26 Aug 2026 00:57:33 +0700 Subject: [PATCH 3/4] docs: ban Python, and keep one working agreement No Python in any form: reaching for it is the tell that a step is being solved by parsing when the tool that owns the answer could just be asked. The near substitutes are ruled out too, and jq is not on macOS to begin with. CLAUDE.md imports AGENTS.md rather than copying it, so there is one source of truth and no per-vendor fork to keep in step. --- AGENTS.md | 16 ++++++++++++++++ CLAUDE.md | 7 +++++++ 2 files changed, 23 insertions(+) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..b2a2dab --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,16 @@ +# Working agreement — solid-layouts + +The operating contract for **any** coding agent working in this repository. Codex, Cursor +and Gemini CLI read `AGENTS.md` natively; Claude Code loads it through the `@AGENTS.md` +import in [`CLAUDE.md`](CLAUDE.md). Never fork these rules into a per-vendor file. + +**JavaScript/TypeScript** monorepo (`packages/`, `Test-UI/`). + +## Invariants (do not break these) + +- **No Python.** Not a script, not `python3 -c`, not a heredoc. Reaching for it is the + tell that a step is being solved by parsing when the tool that owns the answer could + just be asked. Do not swap it for another parser either, and do not assume `jq` is + present: it does not ship with macOS. A fixed-shape field is one `sed -nE` line; + anything needing real parsing belongs in TypeScript, where it can be tested. If a task seems + to need Python, the approach is wrong. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..6190b41 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,7 @@ +@AGENTS.md + +# Claude Code notes — solid-layouts + +The import above is binding: [`AGENTS.md`](AGENTS.md) is the working agreement for this +repository, and every Claude Code session loads it automatically. Do not copy rules here, +one source of truth, no drift. Only genuinely Claude-specific wiring belongs below. From f7448e2a13c1e356da0d45534129be79736a77a5 Mon Sep 17 00:00:00 2001 From: meh Date: Mon, 31 Aug 2026 15:51:48 +0700 Subject: [PATCH 4/4] fix(test): assert the root handler without fighting SlotAttrs `SlotAttrs` types its values as `string`, so comparing the handler by identity had no matching `toBe` overload and `tsc --noEmit` failed while `bun test` passed. `toHaveProperty(name, value)` still asserts it is the same function, and is the idiom the rest of the file already uses. --- packages/solid-layouts/src/component.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/solid-layouts/src/component.test.ts b/packages/solid-layouts/src/component.test.ts index dc911c8..4349fbd 100644 --- a/packages/solid-layouts/src/component.test.ts +++ b/packages/solid-layouts/src/component.test.ts @@ -123,7 +123,7 @@ describe("defineComponent: the props split", () => { expect(seen.slot?.root).not.toHaveProperty("onInput"); // The contrast that keeps this honest: an undeclared handler is plain // HTML and still has to reach the element. - expect(seen.slot?.root?.onClick).toBe(handler); + expect(seen.slot?.root).toHaveProperty("onClick", handler); dispose(); });