Skip to content

fix(codegen): preserve imported class expression property reads - #9758

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9366-imported-class-expression-prototypes
Closed

fix(codegen): preserve imported class expression property reads#9758
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9366-imported-class-expression-prototypes

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

An imported binding such as export let Expr = class {} returned undefined for Expr.prototype, while the same read worked in its defining module. The imported-variable property path loaded the right value but masked away its class tag before dispatch.

Use the ordinary property dispatcher after loading the binding through its live getter. This preserves class references, heap objects, and primitive values and retains existing origin-name handling for renamed re-exports.

Fixes #9366.

Validation

  • The original two-module reproducer fails before the change. The expanded 21-line fixture matches Node 26.5.1 through the parity harness, including prototype identity, named expressions, namespace and renamed imports, live reassignment, plain objects, strings, accessors, and non-invocation of imported functions during property reads.
  • The existing renamed-class namespace parity fixture also passes.
  • All 1,411 codegen tests pass (1 ignored), including a new check that the renamed live getter runs once and its intact value bits reach class-property dispatch.
  • The affected-crate runner passes 3,137 runtime tests (4 ignored) and 1,083 compiler tests before stopping at the existing PERRY_CONCAT_SITE_CACHE registration failure addressed by fix(cache): register concat switch and explain codegen inputs #9748.
  • Formatting, file-size, and test-registration checks pass. The base's raw-TLS policy failure and Linux pthread declaration warnings are addressed separately by fix(gc): audit raw TLS holders and pin census snapshot lifetime #9750 and fix(runtime): use typed pthread attributes for Linux stack bounds #9752.

Additional probes confirmed separate pre-existing gaps in exported anonymous-class name inference and descriptor-value/prototype identity inside the defining module; this change addresses imported property dispatch.

No version bump.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed imported class-expression bindings so .prototype reads return the correct value across modules.
    • Preserved class identity and value types through renamed and namespace re-exports.
    • Ensured live bindings reflect reassigned exported class expressions.
  • Tests

    • Added coverage for prototype identity, rebinding behavior, computed property access, and re-export scenarios.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: fc0cd492-4580-4552-89cd-ecbcad975a14

📥 Commits

Reviewing files that changed from the base of the PR and between 12efed1 and fb0f032.

📒 Files selected for processing (6)
  • changelog.d/9758-imported-class-expression-prototypes.md
  • crates/perry-codegen/src/expr/property_get.rs
  • crates/perry-codegen/src/expr/property_get/tests.rs
  • test-files/_helpers/imported_class_expr_9366.ts
  • test-files/_helpers/imported_class_expr_barrel_9366.ts
  • test-files/test_gap_9366_imported_class_expression_prototypes.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The code generator now preserves tagged values when reading properties from imported exported variables. New unit and integration tests cover class-expression prototypes, renamed re-exports, namespace access, and live rebinding.

Changes

Imported class-expression prototype reads

Layer / File(s) Summary
Generic dispatch for imported variables
crates/perry-codegen/src/expr/property_get.rs, crates/perry-codegen/src/expr/property_get/tests.rs
Imported variables now use generic property dispatch after loading their live getter value. The regression test verifies one getter call and preservation of the value tag.
Cross-module prototype validation
test-files/_helpers/imported_class_expr_9366.ts, test-files/_helpers/imported_class_expr_barrel_9366.ts, test-files/test_gap_9366_imported_class_expression_prototypes.ts, changelog.d/9758-imported-class-expression-prototypes.md
Test fixtures and integration tests cover class declarations, class expressions, barrel aliases, prototype identity, computed access, and export rebinding. The changelog records the fix.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to fb0f0

Imported class-expression property reads now preserve class values and correctly expose prototypes across module boundaries, including renamed and live bindings. The covered behavior is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (1 skipped: 1… 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 concisely describes the primary codegen fix for imported class-expression property reads.
Description check ✅ Passed The description is mostly complete. It explains the problem, implementation, related issue, validation results, known unrelated failures, and version status. It uses a Validation section instead of th…
Linked Issues check ✅ Passed The changes address issue [#9366]. Imported class-expression bindings now use the live getter and ordinary property dispatcher, preserving .prototype reads for class expressions and named class expr…
Out of Scope Changes check ✅ Passed The changed code, regression tests, helper fixtures, and changelog entry support the imported class-expression property-read fix. The broader value-preservation tests validate the generalized dispatch…
Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9798 (rebase-merged, so your commits keep their authorship). Thanks!

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.

Exported class EXPRESSION reads .prototype as undefined from an importing module (class declarations are fine)

1 participant