fix(checker): JSX children become part of the component contract - #8314
Conversation
A Jac component receives JSX children only if it happens to declare a parameter literally named `children`. Nothing checked that, so the failure mode was a blank page with no compile error, no console error and a clean `jac check` -- while the spelling nobody writes, `children="explicit"` as an attribute, has always been an error. Two checks, one contract: W1053 fires at the call site when an element passes children to a component that declares none. Comments and whitespace-only text are not children (the same filter every backend applies when lowering a body), and a lone `props` parameter is exempt because the codegen passes that object through whole, so children arrive as `props.children`. E1108 fires on the declaration when a `pages/` project's exported `app` declares no `children`. That case cannot be caught at a call site: the children come from generated JavaScript, `createElement(app, null, <routes/>)` in `_entry.js`, which the checker never sees. An `app` that ignores them drops every route, which is why it was documented as a footgun rather than checked. Along the way, `_validate_component_jsx_props` stops bailing out on zero-parameter components, so `<NoParams bogus="x"/>` is an E1101 like it is on every other component, and `_validate_jsx_props` stops returning early on elements with no attributes -- an element with children but no attributes is exactly the case that needed checking. `pages/` as a project-shape fact now lives in `jaclang/project/pages_routing.jac`, which both the type checker and `RouteScanner` read, so the directory name has one definition.
… element
`Route` wrapped an `element=` route in a zero-argument, props-ignoring
lambda. `@solidjs/router` delivers a parent route's matched child as that
component's `props.children`, so the child was dropped on the floor, and
Solid's `AuthGuard` had no `<Outlet/>` fallback to recover it: a
`<Route element={<AuthGuard/>}>` with nested routes rendered nothing.
React survives the same shape through its `<Outlet/>` fallback. Solid's
`component=` form and the generated `pages/` entry both pass
`props.children` explicitly, which is why no test caught this.
The lambda now takes the route props and publishes them as the outlet
context the shim already uses for layouts, so both a nested `<Outlet/>`
and the guard's own fallback resolve the matched child.
Two defects surfaced while pinning it down, both on the same path:
`Outlet` compared its `dict`-typed `props` against `undefined`, which
lowers to the structural dict-equality helper and therefore ran
`Object.keys(undefined)`. Every `<Outlet/>` outside an outlet context
threw rather than rendering nothing.
`Route` read `props.element` to pick its branch. Under the Solid JSX
transform that attribute is a getter, so reading it instantiated the
element for every declared route at router-config time -- a logged-out
user hit `Navigate` from guards on routes that never matched. The branch
now tests for the key and reads the value inside the lambda, where the
route actually renders.
`AuthGuard` falls back to `<Outlet/>` when it has no children, matching
React and react-native.
…suite fail Two of the three coverage gaps jaseci-labs#8260 lists, plus the one durable fix that keeps the third from recurring. A jsdom scenario drives the real generated `pages/` entry -- the follow-up jaseci-labs#8134 deferred, and the only coverage that can reach either defect on that path. Both halves were checked by reintroducing the defect: - `auth_redirect` reaches the guard. With the positional `AuthGuard("/signin")` form restored, the scenario lands on `/login` and renders nothing. The fixture's redirect target is deliberately not the default and is a real route, so a dropped redirect is a visibly wrong page rather than a subtly wrong string. - The `app` wrapper renders the route tree it is handed. With `{children}` removed from the wrapper, every route disappears while the shell still mounts. `test_client_integration_ci_coverage.jac` fails when a `JAC_CLIENT_INTEGRATION`-gated suite is named by no workflow. Such a suite skips itself everywhere else, so the ungated lane collects it and reports a pass without executing a line -- that is how `test_react_state_semantics.jac` went unrun from the day it was written. `ci.yml` already carried a comment saying as much, and a comment cannot fail. The audit it encodes found three more: `test_client_bundle_retry.jac`, `test_serve_client.jac` and `test_preact_e2e.jac`, none named by any workflow, and all three red on a clean checkout for reasons of their own (a server that exits instead of surviving a broken bundle, 8 failing cases against a real Vite/Bun build, a scaffolded jac.toml with a duplicated `[client]` table). Naming them in CI would only turn the lane red, so they are quarantined in writing with the defect that blocks each, and a second test keeps that list from going stale.
…clare
W1053 had to predict what the codegen would do with `props`, because the
rule that decides it -- `_component_call_abi` -- is re-derived by every
pass that needs it, from a different view of the parameter list each
time. Testing that premise found it already broken, on a shape the
checker accepted without a word:
def Card(props: any, tone: str = "flat") -> JsxElement
function Card(props) {
const {props, tone: tone = "flat"} = props;
`node --check` rejects that -- 'props' is already declared -- so the
bundle never parses. The keyword-only spelling is worse for being quiet:
it emits `function Card(props, tone = "flat")`, which the renderer calls
with a single object, so `tone` is always its default and every `tone=`
at a call site is discarded. That is the same silent-drop this issue is
about, in a prop that is not `children`.
E1109 rejects the shape at the declaration, which is where the choice is
made, and blocks codegen because neither lowering is salvageable. It also
makes W1053 sound rather than approximate: a component that declares
`props` now always receives the bundle whole, so children reach it as
`props.children` and the exemption is exact instead of a guess about
parameter counts. Before this, W1053 claimed children were discarded on a
component whose `props.children` rendered fine.
Within the type checker the three checks that reason about a component's
props -- W1052, E1108, E1109 -- now share one `_jsx_component_params`,
so they cannot disagree about what a component's parameter list is.
The wider fix is still owed: the ABI belongs in one module that codegen
and the checker both call, rather than five sites re-deriving it. E1109
removes the boundary where they demonstrably diverge; it does not remove
the duplication.
|
CI audit from current
A rebase/update onto current |
- Ensure JSX children are validated as part of the component contract, preventing silent drops of children when not declared. - Introduce diagnostics W1053 and E1108 to catch cases where components drop children or where a `pages/` project app does not declare children. - Reject props bundles declared alongside other parameters with E1109, ensuring valid JavaScript output. - Update tests to cover new validation rules and ensure proper routing behavior in both React and Solid frameworks. - Add new fixtures for pages without routes to validate manual routing scenarios.
|
Follow-up on the current head |
…onent-contract # Conflicts: # .github/workflows/ci.yml # jac/jaclang/client/impl/route_scanner.impl.jac # jac/jaclang/client/impl/solid_runtime.impl.jac # jac/jaclang/compiler/backends/es/impl/esast_gen_pass.impl.jac # jac/jaclang/compiler/passes/impl/type_checker_pass.impl.jac # jac/jaclang/compiler/passes/main/type_checker_pass.jac # jac/jaclang/compiler/types/type_evaluator.impl/jsx_type_check.impl.jac # jac/tests/compiler/backends/es/fixtures/component_routing_return_types.jac # jac/tests/compiler/passes/fixtures/checker/checker_jsx_children_contract.jac # jac/tests/compiler/passes/fixtures/checker/checker_jsx_props_bundle_exclusive.jac # jac/tests/compiler/passes/fixtures/checker/checker_jsx_required_props.jac # jac/tests/runtimelib/test_react_auth_jsdom.jac
…he old path The merge moved these fixtures to tests/compiler/passes/fixtures/pages_routing (matching main's passes/main -> passes flattening) but only staged the additions, leaving stale copies committed at the old path too.
…e checker The merge with main (which rewrote the type checker into a walker-based architecture and dropped the old inference_only split) left three latent bugs in our JSX component-contract feature: - _is_jsx_tag_node assumed the old parser emitted separate open/close-tag JsxElement nodes to filter out; the current parser emits exactly one JsxElement node per JSX literal, so the filter silently skipped validation (W1053, E1103, and related checks) on nearly every element. - JacProgram.compile() lost its type_check kwarg and now always runs full analysis, so the test helper's explicit type_check=True no longer matched the signature. - The same schedule change means program.compile() already runs TypeCheckPass; the tests' extra explicit run_pass(TypeCheckPass, ...) was a genuine second full run. Diagnostics routed through the shared, cached TypeEvaluator (W1053, E1101-E1104) were naturally deduplicated by memoization, but exit_ability's own checks (E1109, W1052, E1108) aren't cache-backed and fired twice. Verified against a clean origin/main checkout that the one remaining failure in this suite (checker_import_missing_module, an off-by-one in an unrelated W1100 import-warning count) predates this branch and is unrelated to the JSX contract work.
…wardRef shape, not a contract violation Component(props, ref: Ref[...]) is the render signature the ES backend lowers to forwardRef((props, ref) => ...) -- ref is the 2nd positional argument React threads in, never a destructured prop. The props-bundle exclusivity check didn't know about this pattern and flagged it as a component declaring a props bundle alongside another parameter.
AuthGuard's `return Outlet({})` fallback (added for the layout-guard
shape) exposed three bugs that CI caught:
- AuthGuard was still typed to return non-optional JsxElement, but
Outlet can return None -- widen it to JsxElement | None to match.
- Route's `elif props["element"] != undefined` read a Solid reactive
getter just to check presence, eagerly invoking the wrapped
component before it was mounted inside the router's outlet
context/Route. Switched to a plain `"element" in props` check so
the component only evaluates where it's meant to.
- `_jac.dict.eq` called Object.keys() on both operands unconditionally,
so comparing a dict against undefined/null crashed instead of
returning false. Guarded both sides.
`_has_rendered_jsx_children` already credits nested content to `children` and ignores comments and whitespace. The older `if expr.children` line still counted a comment-only body, so a required `children` went unreported exactly where W1053 said nothing was passed.
The new diagnostics extend behaviour that existing tests already cover, so their cases move there instead of adding parallel fixtures: - E1101/E1102 at attribute-less and zero-parameter call sites, and W1053, join the step-3 JSX prop-check fixture. Required-prop checking had no test before, which is how the attribute-less gate went unnoticed. - E1109 joins the props-bag fixture that already declares both bundle styles. - JsxPage/JsxLayout lowering joins the union-return codegen fixture; the predicate is shared, so one spelling each is enough. - E1108 keeps one dropping and one rendering pages/ project. The no-route and no-pages cases are the route scanner's tests and stay there. - The Solid element= guard scenarios run through the existing routing harness with a second entry, instead of a third copy of the jsdom boot and install loop. - The React pages/ jsdom test exercised unchanged code and is dropped.
`return_type_is_jsx` and `is_jsx_runtime_helper` are unitree predicates shared by the type checker and the ES backend. symbol_utils is where the compiler keeps those; a two-function module at the package root is not.
A one-line wrapper around startswith and a glob for a three-name tuple were used at two and one sites. Write the checks where they are read.
…at build time E1108 had the type checker re-derive "is this the entry of a routed pages/ project" from disk: a config lookup, then a parse of every pages/*.jac, with a narrower route rule than the scanner (bare names only, project root only, jac.toml required). The bundler already knows all of that when it generates the entry that renders app with the routes as children, and the compiled manifest already records the exported app's parameter names, so the check moves there and reads the manifest. A warm build stays a cache hit. jaclang/project/pages_routing and the checker-side diagnostic go away; the route scanner keeps its own helpers.
|
I pushed six commits onto this branch. Here is what each one changes and why, in plain terms. 1. Merge main into the branch 2. Drop the leftover 3. Fold the new tests into the tests that already own the behaviour
4. Move the shared JSX predicate into 5. Inline the runtime-helper and return-type literals 6. Move the pages-entry check from the checker to the bundler Net for the PR against main: 31 files, +469 / -93, down from +1189 / -86. Every diagnostic and the Solid fix is still proven by a running test (checker JSX 9, codegen 5, route scanner 13, client warm start 2 including the end-to-end refusal, mobile target 54, Solid routing jsdom with 7 scenarios). |
jac fmt reflowed four files. test_backend_purity counts symbol_utils mentions in the backends: return_type_is_jsx reads the declared return annotation, which is the declaration-surface read the test sanctions, so calls.impl.jac goes from 3 to 4 and decls.impl.jac gets its entry.
|
Re-validated the final head live against main 0.37.7 (checker probe on 20 call-site shapes vs the same probe on main, bundler probe on ten project shapes including toml-less, One residual, filed as #9015 so it is not lost with the merge: the hot-reload path recompiles the edited module without running the new entry check, so editing |
…9002 PR A) The first sentinel fixture, and the first time the fixture-side assert count moves. conversation_param.jac asserted twelve things about itself and then printed "CONV_PARAM_PASS"; the test greped for that string. Twelve real checks behind one boolean, with the runner unable to say which one failed. All twelve are now visible assertions in test_conversation.jac, across three tests named for what they establish: the caller's list is mutated in place, not replaced id(history) unchanged; a pre-seeded dict survives the round trip scaffolding and finish_tool are filtered out, a real tool result is not no system message; exactly one tool result, keeping its name and its tool_call_id; no finish_tool leaking under any role a second turn grows the same list and keeps every earlier message the list grows; three user messages across both turns; the pre-seeded and turn-one messages both survive Two assertions are new: the values the calls returned. The fixture printed a sentinel and never checked them, so a by-llm() that wrote history correctly and answered wrongly would have passed. Also here, because the branch picked up checker fix jaseci-labs#8314 from main: that made MTRuntime(messages=[Message(...)]) an E1053 and broke jac check on four pre-existing constructions in test_byllm.jac. They become mk_run() calls, which is where this refactor was taking them anyway. Verified those errors exist on the branch without this commit's changes. Two of the four passed resp_type=None. mk_run does not accept that, and widening it only moves the error inward: MTRuntime's own resp_type is annotated `type` while callers pass None at runtime. Both tests are about tool resolution and are indifferent to the response type, so they take the default. The annotation mismatch in MTRuntime is left alone and noted for jaseci-labs#9002. Ledger: fixtures 45 to 44. Fixture-side asserts 66 to 53. Tests 199 to 201, one sentinel test out and three real ones in. Inline MTRuntime constructions 4 to 0. Suite 201 passed, 1 skipped.


Two defects, one root cause: a component receives only what it declares. Children handed to a component that never declares
childrenwere dropped silently, a blank render behind a cleanjac check. Fixes #8260, follow-up to #8134.The mechanism
The client codegen destructures a component's declared parameter names out of
propswith no rest element, so anything undeclared has nowhere to land.flowchart LR CS["call site passes<br/>title + nested children"] --> P["props = { title, children }"] P --> D["emitted:<br/>const { title } = props"] D --> OK["title bound"] D -.->|"no rest element"| X["children dropped<br/>no runtime signal"]The codegen is deliberately left as is. This PR makes the contract visible instead of passing children through, so a component still receives exactly what it declares.
The deeper gate
The checker never looked at the call sites where this shows up.
_validate_jsx_propsreturned early when the call site passed no attribute, and_validate_component_jsx_propsreturned early when the component declared no parameter. So<Card/>againstdef Card(title: str)checked clean,<Zero bogus="x"/>against a zero-parameter component checked clean, and<Card><Leaf/></Card>never reached validation at all. Both early exits are removed; every call site is validated like any other. This can surfaceE1102andE1101on code that previously passed.The contract
flowchart TD A{"return type names JsxElement,<br/>JsxPage or JsxLayout?"} A -->|no| Z0["not a component, nothing checked"] A -->|yes| B{"parameter shape"} B -->|"'props' plus other params"| E9["E1109 error<br/>on the declaration"] B -->|"'props' alone"| Z1["fine, children arrive<br/>as props.children"] B -->|"declares 'children'"| Z2["fine"] B -->|"no 'children'"| C{"who passes the children?"} C -->|"a Jac call site"| W["W1053 warning<br/>at the call site"] C -->|"the generated pages/ entry"| E8["client build refuses<br/>the entry"]W1053(checker)E1109(checker)propsbundle beside another parameterClientBundleError(bundler)pages/appthat would discard the whole route treeNested content counts toward a
childrenparameter only when something is rendered: whitespace and{#* comments *#}do not count, forW1053and forE1102alike.Component-hood comes from one predicate shared with the codegen,
return_type_is_jsxinsymbol_utils, soJsxPage,JsxLayoutand unions are covered exactly asJsxElementis.E1109concretely:Solid:
element=parent routesThe mirror image of the React defect, and the same blank page with no error anywhere.
flowchart TD subgraph B["before"] B1["Route with element={Guard}"] --> B2["wrapped in a<br/>props-ignoring lambda"] B2 --> B3["matched child never reaches the guard"] B3 --> B4["parent-route guard renders nothing"] end subgraph A["after"] A1["Route with element={Guard}"] --> A2["wrapper provides the matched child<br/>through the outlet context"] A2 --> A3["AuthGuard falls back to Outlet<br/>only when given no children"] A3 --> A4["parent-route and layout forms both render"] endAlso in this PR
W1052(untypedprops: anybag) now uses the shared predicate, so it also fires on components returningJsxPage,JsxLayoutor a union; before it fired onJsxElementonly._jac.dict.eqin the JS runtime guards null and non-object operands. The ES backend lowersx != Noneloosely but notx != undefined, so a dict compared toundefinedreachedObject.keys(undefined)and threw. Found by the Solid jsdom test.Tests
Cases live in the tests that already own the behaviour; the diagnostics extend existing checks rather than add new areas.
jsx_step3_jsx_prop_check(extended)E1102on<Plain/>and on a requiredchildrenwith nothing or only a comment nested;E1101on a zero-parameter component;W1053on two components declaring nochildren; nested content satisfieschildrenjsx_props_bag_untyped_and_typed(extended)E1109onpropsbeside another parameterunion JsxElement return type still triggers props-destructure lowering(extended)JsxPageandJsxLayoutcomponents lower through the props protocol like unions dopages entry that ignores children drops every route(route scanner)children, loneprops, and no clientappa pages entry whose app cannot receive the route tree is refused(client warm start)apptakes nochildrensolid file-based routing mounts, navigates, and guards in jsdom(extended)/nestedlogged in and out,/layout-guarded,/bareDocs:
jac-cl-components.mdgains the children contract and both codes,jac-cl-routing.md's "silently drops every route" footgun becomes the build refusal it now is,jac-cl-auth.mdgains the Solid direction of the same bug, and the components tutorial's "childrenmust have a default" warning becomes a tip that says why.Not in scope
React-native
AuthGuardrender coverage (issue task 4, second bullet).react_native_multiscreen's test still asserts onlyreturncode == 0plus an APK directory; a real render assertion needs a react-native renderer in CI.