Skip to content

fix(compile): omit erased names from module namespaces - #8994

Merged
proggeramlug merged 2 commits into
PerryTS:mainfrom
proggeramlug:fix/8904-namespace-materialization
Aug 28, 2026
Merged

fix(compile): omit erased names from module namespaces#8994
proggeramlug merged 2 commits into
PerryTS:mainfrom
proggeramlug:fix/8904-namespace-materialization

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • derive module namespace keys from consumer-visible export names so private backing functions such as _null do not leak beside their public aliases
  • exclude erased TypeScript interfaces and type aliases when materializing dynamic and nested namespace objects
  • add a Zod-shaped regression for import * as z; export { z }, nested export * as coerce, reflection, alias-only exports, and type erasure

Verification

On root@perrymaster.skelpo.net:

  • cargo fmt --all -- --check
  • cargo build --release -p perry
  • cargo build --release -p perry-runtime-static -p perry-stdlib-static
  • ./run_parity_tests.sh --filter namespace_type_erasure_8904
  • ./run_parity_tests.sh --filter export_star_as_namespace_object
  • ./run_parity_tests.sh --filter namespace_import_enumerable
  • real Zod 4.3.5 no-cache compile: Object.keys(z).length === 236 in both Node and Perry, z.coerce is owned/enumerable, and z.coerce.number()("42") === 42

No version bump.

Fixes #8904

Summary by CodeRabbit

  • Bug Fixes

    • Fixed namespace exports so type-only declarations are excluded from runtime namespace objects.
    • Prevented private backing names from appearing as public exports.
    • Ensured nested and dynamically imported namespaces expose only valid runtime values.
  • Tests

    • Added coverage for namespace re-exports, enumerable properties, runtime keys, and numeric coercion behavior.

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 27 minutes.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3f206d76-5bf8-4a5c-9b65-bcd0aab550e8

📥 Commits

Reviewing files that changed from the base of the PR and between 9c7d1fc and b9afb22.

📒 Files selected for processing (1)
  • changelog.d/8994-namespace-erased-names.md

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fcb4214b-bae0-4e7a-8b83-f1d4ad487ea1

📥 Commits

Reviewing files that changed from the base of the PR and between 4b9dde9 and 9c7d1fc.

📒 Files selected for processing (5)
  • crates/perry/src/commands/compile/run_pipeline.rs
  • test-files/issue_8904_namespace_materialization/coerce.ts
  • test-files/issue_8904_namespace_materialization/external.ts
  • test-files/issue_8904_namespace_materialization/index.ts
  • test-files/test_gap_namespace_type_erasure_8904.ts

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


📝 Walkthrough

Walkthrough

The compiler now excludes type-only declarations and private backing names from runtime exports and namespace entries. New regression fixtures verify aliased values, nested namespaces, enumerable properties, and erased declarations.

Changes

Namespace type erasure

Layer / File(s) Summary
Runtime export filtering
crates/perry/src/commands/compile/run_pipeline.rs
Adds type-only binding detection. Runtime export population now uses public export names and skips erased declarations.
Namespace entry filtering
crates/perry/src/commands/compile/run_pipeline.rs
Dynamic-import and nested-namespace entries now omit type-only bindings.
Namespace regression coverage
test-files/issue_8904_namespace_materialization/*, test-files/test_gap_namespace_type_erasure_8904.ts
Adds aliased runtime values, erased declarations, a nested namespace, and checks for the expected runtime namespace shape.

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

Merge Risk: ⚪ Minimal · up to 9c7d1

Compiled module namespaces will omit private backing names and erased TypeScript declarations while preserving runtime exports and aliases. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly and concisely describes the main change: excluding erased names from module namespace objects.
Description check ✅ Passed The description explains the changes, identifies issue #8904, lists verification commands and results, and confirms no version bump. It uses a "Verification" heading instead of "Test plan" and omits t…
Full details: Description check

Explanation

The description explains the changes, identifies issue #8904, lists verification commands and results, and confirms no version bump. It uses a "Verification" heading instead of "Test plan" and omits the checklist, but it is otherwise complete and relevant.

✨ 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

Merged, validated as a batch of six on current main.

Validation — runtime 2786/0, parser 40/0, codegen 1341/0 (RUST_TEST_THREADS=1); scripts/run_lint_gates.sh 57 of 58 including the compile tier (cargo check --workspace --all-targets under -D warnings, and clippy) — the exception is the pre-existing Actions-expression artifact (#8929). All six stacked without conflict.

One fix pushed: this PR touches crates/ with no changelog fragment and no skip-changelog label, so the changeset gate in lint would have rejected it. Added one. Four of the six PRs in this batch had the same gap.

@proggeramlug
proggeramlug merged commit d4ec70c into PerryTS:main Aug 28, 2026
19 checks passed
@proggeramlug
proggeramlug deleted the fix/8904-namespace-materialization branch August 29, 2026 05:46
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.

Namespace import object is not materialised: Object.keys(ns) is empty and some members (zod's z.coerce) are undefined, while others work

1 participant