fix(runtime): inherit static data fields from a parent class - #6443
Merged
proggeramlug merged 2 commits intoJul 16, 2026
Conversation
A subclass inherits its parent's static data properties via the class-object prototype chain (`Sub.__proto__ === Base`) — both in-body `static x = …` fields and runtime `Base.x = …` assignments. Perry inherited static METHODS (they walk the class_id chain) but the static-DATA-field read only consulted the receiver class's own `CLASS_DYNAMIC_PROPS`, so `Sub.x` returned `undefined` even though `Base` defined `x`. `get_field_by_name` now walks the `get_parent_class_id` chain after an own-field miss, reading each ancestor's `CLASS_DYNAMIC_PROPS` (and honoring a deleted key on an ancestor). This mirrors the existing static-method chain walk. Surfaced by Auth.js v5: `SignInError.kind = "signIn"` is read off a `CredentialsSignin` subclass (`this.constructor.kind`) to choose the sign-in vs error redirect page; the missing inheritance sent every credentials error to `/auth/error` instead of `/auth/login`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughStatic class-reference property lookup now traverses ancestor class IDs for inherited dynamic static data fields. Regression tests cover direct and deep inheritance, subclass shadowing, and lookup past a deleted intermediate field. ChangesStatic field inheritance
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs (1)
855-889: 📐 Maintainability & Code Quality | 🔵 TrivialRebuild static wrapper crates and use local compiler feedback.
As per coding guidelines, when changing runtime or stdlib code, rebuild the corresponding static wrapper crates (
perry-runtime-staticandperry-stdlib-static) because the runtime and stdlib crates themselves emit only rlibs and otherwise may leave stale archives linked. Additionally, usecargo check -p perryfor fast local compiler feedback, thencargo build --profile perry-dev -p perry; reserve release/dist profiles for shipping or optimization-sensitive work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs` around lines 855 - 889, After updating the static-field lookup logic around CLASS_DYNAMIC_PROPS, rebuild both perry-runtime-static and perry-stdlib-static so their archives are refreshed. Run cargo check -p perry for fast compiler feedback, then cargo build --profile perry-dev -p perry; do not use release or dist profiles for this change.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs`:
- Around line 877-879: In the prototype-chain traversal in get_field_by_name,
replace the break used when class_is_key_deleted(cid, name) returns true with
continue. This should skip the deleted property on the current ancestor while
allowing lookup to proceed to higher ancestors.
---
Nitpick comments:
In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs`:
- Around line 855-889: After updating the static-field lookup logic around
CLASS_DYNAMIC_PROPS, rebuild both perry-runtime-static and perry-stdlib-static
so their archives are refreshed. Run cargo check -p perry for fast compiler
feedback, then cargo build --profile perry-dev -p perry; do not use release or
dist profiles for this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac87bdec-2bcd-4582-a8e6-ea07d0649c1e
📒 Files selected for processing (2)
crates/perry-runtime/src/object/field_get_set/get_field_by_name.rstest-files/test_gap_static_field_inheritance.ts
…nt-chain walk The static-data-field inheritance walk broke out of the loop when a key was marked deleted on an intermediate ancestor. But `delete Mid.foo` only removes Mid's own static — a higher ancestor may still define it, so `Sub.foo` must inherit `Base.foo`, not resolve to undefined. `break` aborted the whole traversal; `continue` skips the deleted level and keeps walking up. Safe: `cid` and `depth` both advance at the top of every iteration, so it can't loop. Verified against node: `delete Mid.tag; Sub.tag` -> "base" with the fix, "undefined" without it. Extended test_gap_static_field_inheritance.
Contributor
Author
|
Applied CodeRabbit's The parent-chain walk aborted on a key deleted at an intermediate ancestor, but |
proggeramlug
added a commit
that referenced
this pull request
Jul 18, 2026
…istry props (#6552) (#6556) * fix(runtime): per-evaluation inherited static wins over last-wins registry props (#6552) A subclass of a class-EXPRESSION value evaluated more than once (`function make(a){return class{static ast=a}}`, then `class Number$ extends make(x) {}` / `class Widget$ extends make(y) {}`) records THIS evaluation's parent object as its static prototype (`class_prototype_object`, #1788). But #6443's static-DATA-field inheritance walk reads the parent's `CLASS_DYNAMIC_PROPS`, which are keyed by the class-expression TEMPLATE id — shared, last-wins across every evaluation — and it ran BEFORE the per-evaluation prototype-object walk. So every renamed effect-Schema class read via `import * as M` (`M.Number.ast`/`M.Widget.ast`) collapsed to the LAST `make(...)`'s static `ast` (the direct export's `DirectKeyword`), re-breaking effect Schema decode (test_gap_renamed_class_export_namespace, the #1758/#1812 guard). Interleave the two walks: at each level of the class-object proto chain, consult the class's pinned per-evaluation parent object BEFORE the parent's registry props. The pinned object is authoritative (it carries this evaluation's own edge); the registry read stays the fallback for a plain declaration parent (#6443: Auth.js `SignInError.kind`), which has no pinned object. Depth order is preserved, so a closer registry static still shadows a deeper per-evaluation one. test_gap_renamed_class_export_namespace is byte-identical to node again across renamed (global-colliding + not) and direct exports; #6443 static-field inheritance + deleted-key fallthrough, #6438 per-eval statics, effect factory static dispatch, and the rest of the class-expr/static suite (18/18) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(runtime): return a present `null` inherited static, don't fall through (#6552 review) CodeRabbit review follow-up. The interleaved per-evaluation proto-object read guarded on `!is_undefined() && !is_null()`, so a static explicitly set to `null` on one evaluation (`class NullAst$ extends make(null) {}`) was treated as "absent" and fell through to the parent's last-wins `CLASS_DYNAMIC_PROPS`, returning a SIBLING evaluation's `ast` instead of `null` (verified: perry read `{_tag:'OBJ'}` where node reads `null`). `null` is a present, authoritative value — only `undefined` means "absent here". Guard on `!is_undefined()` alone so a present `null` is returned and only a true miss continues to the registry / a higher ancestor. Extends the gap test + helper with a renamed null-static export (`M.NullAst.ast === null`), placed before the last non-null `make(...)` so `null` is a genuine discriminator; byte-identical to node. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ralph <ralph@skelpo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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
A subclass inherits its parent's static data properties through the class-object prototype chain (
Sub.__proto__ === Base) — both in-bodystatic x = …fields and runtimeBase.x = …assignments. Perry inherited static methods (they walk the class_id chain vialookup_static_method_in_chain) but the static-data-field read only consulted the receiver class's ownCLASS_DYNAMIC_PROPS, soSub.xreturnedundefinedeven thoughBasedefinedx.get_field_by_namenow walks theget_parent_class_idchain after an own-field miss, reading each ancestor'sCLASS_DYNAMIC_PROPS(and honoring adeleted key on an ancestor). An own field still shadows an inherited one.Why it matters
Surfaced by Auth.js v5:
SignInError.kind = "signIn"is read off aCredentialsSigninsubclass (this.kind = this.constructor.kind) to choose the sign-in vs error redirect page. The missing inheritance sent every credentials error to/auth/errorinstead of/auth/login.Testing
test_gap_static_field_inheritance.ts(byte-compared to Node): inherited in-body field, inherited externalC.x = …assignment, 2-level inheritance, own-field-shadows-inherited, and that shadowing a subclass doesn't mutate the parent. Existing perry-runtime suite passes (the rotating URL/stream failures are pre-existing parallel-execution flakes).https://claude.ai/code/session_01NjymZygYh31wM9h3wLnUqv
Summary by CodeRabbit
Bug Fixes
undefinedwhen accessed via subclasses.Tests