Skip to content

fix(hir): dispatch Array-mutator-named methods on any-typed receivers by runtime shape (#5139) - #5194

Merged
proggeramlug merged 1 commit into
mainfrom
fix/5139-renderToStaticMarkup
Jun 15, 2026
Merged

fix(hir): dispatch Array-mutator-named methods on any-typed receivers by runtime shape (#5139)#5194
proggeramlug merged 1 commit into
mainfrom
fix/5139-renderToStaticMarkup

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5139react-dom/server's renderToStaticMarkup returned an empty string instead of the rendered HTML when react-dom is compiled natively via perry.compilePackages.

Root cause

The bug was in HIR lowering, not in react-dom. The Fizz SSR renderer accumulates output by calling destination.push(chunk) on a plain-object sink:

const destination = { push(chunk) { result += chunk } };
// …flows through several any-typed params…
function writeChunk(d, chunk) { return d.push(chunk); } // d : any

HIR lowering eagerly rewrote recv.push(x) — where recv is statically any — into the array-only fast path (Expr::ArrayPush / the array.push_single native arm). That path reads the receiver's header as an ArrayHeader, so the plain object's bytes were interpreted as array length/capacity: push returned a bogus numeric length and the user closure never ran, dropping every chunk — SSR yielded "".

This affected any object owning a method whose name collides with an Array.prototype mutator (push/pop/shift/unshift/splice/sort/reverse/concat) when invoked through an any-typed receiver.

Fix

The two lowering sites — local_array_methods.rs (bare-ident receivers) and array_only_methods.rs (the fallback for arbitrary expressions) — now defer those mutator names to the runtime js_native_call_method dispatch when the receiver's static type is unknown/any. That dispatcher already selects by runtime shape:

Positively array-typed receivers keep the inline fast path unchanged, and the read-only iteration folds (map/filter/entries/…) on any receivers are untouched.

Verification

  • Reproduced against the real react-dom@18.3.1 package compiled via perry.compilePackages. The issue's repro now prints, matching Node:
    <ul id="L"><li>item-1</li><li>item-2</li><li>item-3</li></ul>
    
  • New regression tests in crates/perry/tests/issue_5139_object_arraylike_method_dispatch.rs:
    • object push closure runs when invoked through any-typed params (the reduced Fizz flush loop);
    • all eight mutator-named own methods dispatch to the object's closures;
    • any-typed real arrays keep Array.prototype semantics, including 1000-push growth across a function boundary, unshift/push/reverse/splice, comparator sort, and in-place pop/shift.
  • Full perry integration suite + perry-runtime unit tests (1035) pass, plus perry-hir/perry-codegen tests — no regressions.

No version bump or CHANGELOG entry (left to maintainer at merge).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed method dispatch for objects with unknown types to correctly invoke custom methods instead of incorrectly treating them as arrays; resolves issue #5139 affecting react-dom/server's renderToStaticMarkup.
  • Tests

    • Added regression tests verifying proper method dispatch for plain objects, array mutators on objects, and real arrays with correct semantics.

… by runtime shape (#5139)

react-dom/server's `renderToStaticMarkup` returned an empty string. The Fizz
renderer accumulates HTML by calling `destination.push(chunk)` on a plain-object
sink `{ push(chunk) { result += chunk } }` that flows through several `any`-typed
params before `writeChunk(destination, …)` invokes it.

HIR lowering eagerly turned `recv.push(x)` — where `recv` is statically `any` —
into the array-only fast path (`Expr::ArrayPush` / the `array.push_single` native
arm). That reads the receiver's header as an `ArrayHeader`, so the plain object's
bytes were interpreted as array length/capacity: `push` returned a bogus numeric
length and the user closure never ran, dropping every chunk (SSR yielded "").

The two lowering sites (`local_array_methods.rs` for bare-ident receivers,
`array_only_methods.rs` for the fallback) now defer the mutator names that a
plain object can legitimately own as a closure property — push/pop/shift/unshift/
splice/sort/reverse/concat — to the runtime `js_native_call_method` dispatch when
the receiver's static type is unknown/`any`. That dispatcher already selects by
runtime shape: a real array hits the dense `js_array_*` helpers (growth still
resolves via the #233 forwarding pointer), while a plain object with an own
callable of that name invokes it with `this` bound to the receiver. Positively
array-typed receivers keep the inline fast path unchanged, and the read-only
iteration folds (map/filter/entries/…) on `any` receivers are untouched.

Verified against the real react-dom@18.3.1 package compiled via
`perry.compilePackages`: the issue's repro now prints
`<ul id="L"><li>item-1</li><li>item-2</li><li>item-3</li></ul>`, matching Node.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Two guards are added to the HIR lowering passes (array_only_methods and local_array_methods) to bail out of the array fast-path when the receiver's static type is None, Any, or Unknown for array-mutator method names. A new regression test file adds three end-to-end tests covering plain-object dispatch, all mutator names, and real arrays stored in any.

Changes

Array-mutator dispatch fix for any/unknown receivers

Layer / File(s) Summary
HIR lowering guards for unknown receiver types
crates/perry-hir/src/lower/expr_call/local_array_methods.rs, crates/perry-hir/src/lower/expr_call/array_only_methods.rs
is_arraylike_mutator_method predicate added; is_not_string routing updated to skip the array block for unknown receivers; try_array_only_methods gains an early Err(args) bail-out for None/Any/Unknown receiver types on the eight mutator method names.
Regression tests for issue #5139
crates/perry/tests/issue_5139_object_arraylike_method_dispatch.rs
Adds perry_bin() and compile_and_run() helpers plus three tests: (1) plain-object push closure via any, (2) all mutator names dispatching to object closures, (3) real arrays in any preserving standard mutator semantics.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Poem

🐇 A bunny once hopped through the any-typed land,
Where push on an object was misunderstood, unplanned.
"That's not an array!" cried the guard with a leap,
Now the type-check defers, no false fast-path to keep.
Plain objects and arrays each follow their own—
The correct method found, wherever they roam! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main fix: dispatching array-mutator methods on any-typed receivers by runtime shape, directly addressing issue #5139.
Description check ✅ Passed The description fully covers the template requirements with clear Summary, Changes, Related issue (#5139), and comprehensive Test plan demonstrating verification against real react-dom package and new regression tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/5139-renderToStaticMarkup

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

compilePackages: react-dom/server renderToStaticMarkup returns empty string

1 participant