Skip to content

fix(codegen): publish self namespace re-exports as live accessors (#10160) - #10162

Closed
proggeramlug wants to merge 1 commit into
PerryTS:mainfrom
proggeramlug:fix/10160-self-namespace-dynamic-import
Closed

proggeramlug wants to merge 1 commit into
PerryTS:mainfrom
proggeramlug:fix/10160-self-namespace-dynamic-import

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

export * as Self from "./self" (a module re-exporting its own namespace — OpenCode does this in 265 modules) was undefined when read through a dynamic import() namespace, while the static import resolved. The NestedNamespace entry's value was a plain load of the module's own @__perry_ns_<prefix> global emitted inside the populator that builds that namespace, i.e. before the global is stored. OpenCode's command bootstrap reads it exactly that way (const { InstanceStore } = await import("@/project/instance-store")), so every real command (run, models, serve, TUI) died with Cannot read properties of undefined (reading 'Service').

Changes

  • emit_namespace_populator (helpers.rs): a self-referencing NestedNamespace entry is published as a live binding — flagged live and materialized as a js_closure_alloc_singleton of its __perry_ns_get_<prefix>__<i> wrapper, like LocalVar/ForeignVar. Foreign export * as entries keep the direct load of the target's global.
  • Getter wrapper emission (artifacts.rs): the self case defines the wrapper as a load of @__perry_ns_<prefix> at read time.
  • Regression tests (crates/perry/tests/source_graph_export_regressions/issue_10160.rs): self re-export through a dynamic namespace (destructuring, property read, recursion ns.Self.Self === ns.Self), through a static import * as, and a foreign export * as on a dynamic namespace (unchanged behaviour). changelog.d/10160-self-namespace-dynamic-import.md.

Known separate gap, left alone: mod.Self === StaticBinding is false under Perry (the static import binding and the dynamic namespace are not the same object); ESM makes them identical.

Related issue

Fixes #10160

Test plan

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md (maintainer handles these at merge)
  • My commit follows the loose fix: prefix convention used in the log
  • I've read CONTRIBUTING.md and agree to the Code of Conduct

https://claude.ai/code/session_01GixUo7gwcatEk4kibdeCWF

Summary by CodeRabbit

  • Bug Fixes

    • Fixed self-namespace re-exports so export * as Self correctly resolves through dynamic imports.
    • Preserved live namespace values, including exported classes, values, and recursive namespace identity.
    • Ensured foreign namespace re-exports continue to work correctly.
  • Tests

    • Added regression coverage for static and dynamic self-namespace imports.

…rryTS#10160)

`export * as Self from "./self"` lowered to a NestedNamespace entry whose
value was a plain load of the module's own `@__perry_ns_<prefix>` global —
inside the populator that builds that very namespace, before the global is
stored. The dynamic `import()` namespace therefore carried the key with an
`undefined` value while the static import path still resolved. OpenCode
uses this self re-export in 265 modules and reads it through dynamic
imports in its command bootstrap, so every real command failed with
`Cannot read properties of undefined (reading 'Service')`.

Self-referencing entries are now published like LocalVar/ForeignVar live
bindings: the populator marks them live and hands `js_create_namespace` a
getter singleton, and the getter wrapper loads the namespace global at read
time. Foreign `export * as` entries keep the direct load.

Claude-Session: https://claude.ai/code/session_01GixUo7gwcatEk4kibdeCWF
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0c8adbab-b7ef-4e50-b8ef-8bcb46f3d4da

📥 Commits

Reviewing files that changed from the base of the PR and between 8a058e2 and 149c1a6.

📒 Files selected for processing (5)
  • changelog.d/10160-self-namespace-dynamic-import.md
  • crates/perry-codegen/src/codegen/artifacts.rs
  • crates/perry-codegen/src/codegen/helpers.rs
  • crates/perry/tests/source_graph_export_regressions.rs
  • crates/perry/tests/source_graph_export_regressions/issue_10160.rs

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


📝 Walkthrough

Walkthrough

The code generator now handles export * as Self from "./self" through a live getter wrapper. Regression tests cover dynamic imports, static namespace imports, recursive namespace identity, and foreign namespace re-exports.

Changes

Self-namespace export resolution

Layer / File(s) Summary
Generate live self-namespace bindings
crates/perry-codegen/src/codegen/helpers.rs, crates/perry-codegen/src/codegen/artifacts.rs
Self-referential namespace entries are marked as live bindings. The generator emits a getter wrapper that reads the module namespace global after js_create_namespace returns.
Validate self-namespace access
crates/perry/tests/source_graph_export_regressions.rs, crates/perry/tests/source_graph_export_regressions/issue_10160.rs, changelog.d/10160-self-namespace-dynamic-import.md
Regression coverage checks dynamic and static namespace access, nested namespace values, recursive namespace identity, and foreign namespace re-exports. The changelog records the fix.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 149c1

The self-namespace export behavior is covered by the updated generator and regression tests, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 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 identifies the main change: publishing self-namespace re-exports as live accessors in code generation.
Description check ✅ Passed The description includes the required Summary, Changes, Related issue, Test plan, and Checklist sections. It explains the cause, implementation, regression coverage, known limitation, and verification…
Linked Issues check ✅ Passed Issue #10160 requires self export * as namespace entries to work through dynamic imports, preserve non-self namespace re-exports, and add regression coverage under crates/perry/tests/. The code no…
Out of Scope Changes check ✅ Passed The changes are limited to self-namespace population and getter-wrapper code generation, regression tests for issue #10160, and the related changelog entry. These changes support the linked issue. No …
Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 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 via merge train #10163 (rebase-merged; main b5a82cfeae, tree identical to the train), cherry-picked patch-identical onto 8a058e2053 with the version bump to 0.5.1546. Validation and the CI attribution against main are in #10163.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant