fix(compiler): cross-framework named-slot consistency + Angular nullish attrs#464
Merged
Merged
Conversation
…readOnly
The React ComponentInstance codegen emitted a named-slot fill as a <Tag.name>
child, but the React target consumes named slots as props — so the fill never
reached the consumer and slot content (e.g. Input prefix/suffix) silently
vanished. Align both sides on a node prop (prefix={<>$</>} consumed as
{props.prefix}), mirroring Solid; scoped slots become function props. This also
matches the authored .ink.tsx source and needs no runtime machinery.
Also add readonly→readOnly to REACT_ATTR_MAP so the native control stops emitting
the invalid lowercase DOM prop (React warning) and read-only state applies.
Solid already consumes named slots as props ({props.prefix}, or {props.prefix?.(args)}
when scoped), but the ComponentInstance codegen emitted the fill as a dead <Tag.name>
child, so slot content never reached the consumer. Emit the fill as the matching prop —
a node prop when unscoped, a function prop when it takes args.
Qwik emitted named-slot fills as dead <Tag.name> children while consuming named
slots via <Slot name>, but never tagged the fill with q:slot — so projected
content silently vanished. Rather than add q:slot wrappers, align Qwik with
React/Solid: named slots are props (a node prop, or a function prop when scoped),
consumed as {props.prefix}. Verified Qwik renders the JSX-valued prop inline with
no extra wrapper element. The default slot still projects through native <Slot/>
(Qwik never populates props.children); emitsQwikSlot updated so the Slot import is
dropped when only named slots are used.
A dynamic native attribute was emitted as an Angular property binding ([id]="id()"); when the signal returns undefined, the assignment reflects as the literal id="undefined" (and ?? null can't help — a property binding stringifies null to "null"). Emit non-property attributes as attribute bindings ([attr.id]="(id()) ?? null"), which Angular removes when the value is null. Boolean/value-semantic and special bindings (value, disabled, readonly, style, …) stay property bindings via a KEEP_PROPERTY set, since [attr.disabled]="false" would wrongly render disabled="false". Component @input bindings are unchanged.
…Qwik)
A re-projected named-slot fill — `icon={<Slot name="icon"/>}` — has a SlotPlaceholder
body, which Solid's inlineNode and Qwik's emitNodeInline rendered as the JSX-braced
`{props.icon}`. Inside the attr-value fill (`icon={…}`) that double-wrapped to
`icon={{props.icon}}`, a parse error caught by the lint conformance suite. Solid's
inlineNode now returns the bare slot read (matching React); Qwik gains a namedSlotRead
helper used by both the SlotPlaceholder emit (braced) and the fill (bare).
…egen fixes The compiler changes (React node-prop slots + readonly→readOnly, Angular nullish [attr.*] bindings) alter the generated Input and Button output. Regenerate the affected component snapshots and update the styled-Input assertions to the new node-prop / attribute-binding forms.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #464 +/- ##
==========================================
+ Coverage 94.05% 94.39% +0.34%
==========================================
Files 112 112
Lines 5415 5460 +45
Branches 1862 1896 +34
==========================================
+ Hits 5093 5154 +61
+ Misses 280 269 -11
+ Partials 42 37 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…gets Add unit tests for the named-slot codegen branches introduced/touched by this work — re-projected slot fills (scoped/unscoped, named/default, with/without fallback) across React/Solid/Qwik, plus expr-rewrite expression forms and Angular control-flow/binding branches. Brings the changed files to >90% statement/line/function coverage (and >90% branch on React/Solid/Qwik/expr-rewrite).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rendering the Input "Default" story across all 7 framework Storybooks surfaced codegen inconsistencies: React, Solid, and Qwik silently dropped named-slot content (the
$/USDaddons) and Angular rendered a literalid="undefined". All JSX targets now emit named slots as props — a node prop (prefix={<>$</>}↔{props.prefix}), or a function prop when scoped — matching their own consumption convention; React also mapsreadonly → readOnly, and Angular binds non-property native attributes via[attr.name]="(expr) ?? null"(omitting nullish) while keeping boolean/value props as property bindings. All 7 frameworks now render the Default Input story identically ($ 0.00 USD).Test plan
vp run readypasses locally — note: the pre-existing→ astrooxlint and Angular-SSR@angular/compilermount failures are unrelated to this PR (identical on a clean baseline)vp test core/compilergreen except the 74 pre-existing Astro lint failuresvp test ui/componentsgreen except the pre-existing Angular-SSR mount tests (local@angular/compilertemp-dir resolution)$ 0.00 USD; Angular has noid="undefined"; React has no DOM-prop console warningChecklist
pnpm changeset) for any change to a published package's behavior, API, or types. See docs/release-process.md.AGENTS.mdordocs/*.mdfiles have been updated. See docs/maintenance.md → "Cross-references". (Codegen bug fix — no public API/convention/structure change.)feat(scope): …,fix(scope): …). See docs/conventions.md → "Commit messages".