Skip to content

fix(compiler): cross-framework named-slot consistency + Angular nullish attrs#464

Merged
alexgrozav merged 9 commits into
mainfrom
alexgrozav/input-story-consistency
Jun 14, 2026
Merged

fix(compiler): cross-framework named-slot consistency + Angular nullish attrs#464
alexgrozav merged 9 commits into
mainfrom
alexgrozav/input-story-consistency

Conversation

@alexgrozav

Copy link
Copy Markdown
Member

Summary

Rendering the Input "Default" story across all 7 framework Storybooks surfaced codegen inconsistencies: React, Solid, and Qwik silently dropped named-slot content (the $/USD addons) and Angular rendered a literal id="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 maps readonly → 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 ready passes locally — note: the pre-existing → astro oxlint and Angular-SSR @angular/compiler mount failures are unrelated to this PR (identical on a clean baseline)
  • vp test core/compiler green except the 74 pre-existing Astro lint failures
  • vp test ui/components green except the pre-existing Angular-SSR mount tests (local @angular/compiler temp-dir resolution)
  • Manually verified all 7 Storybook Default stories render $ 0.00 USD; Angular has no id="undefined"; React has no DOM-prop console warning

Checklist

  • Added a changeset (pnpm changeset) for any change to a published package's behavior, API, or types. See docs/release-process.md.
  • If this PR changes public API, build flow, conventions, or repo structure, the relevant AGENTS.md or docs/*.md files have been updated. See docs/maintenance.md → "Cross-references". (Codegen bug fix — no public API/convention/structure change.)
  • Commit messages follow the conventional format (feat(scope): …, fix(scope): …). See docs/conventions.md → "Commit messages".

…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

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.50746% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.39%. Comparing base (287b326) to head (21a2147).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
core/compiler/src/codegen/targets/react/index.ts 95.65% 1 Missing ⚠️
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     
Flag Coverage Δ
shard-1 85.73% <67.16%> (-0.27%) ⬇️
shard-2 90.22% <98.50%> (+0.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…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).
@alexgrozav alexgrozav merged commit 01a5207 into main Jun 14, 2026
10 checks passed
@alexgrozav alexgrozav deleted the alexgrozav/input-story-consistency branch June 14, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant