Skip to content

Conversation

@privatenumber
Copy link
Contributor

@privatenumber privatenumber commented Oct 16, 2025

Fixes #359

Removes two problematic code sections from PR #352:

  1. Lines 26-28: Unnecessary duplicateExports() call that caused superfluous export name duplication in namespace bodies

  2. Lines 596-605: Incorrect removal of export keywords from nested namespaces, which changed their accessibility from public to private

Problem

The previous implementation was removing export keywords from nested namespaces, which:

  • Changed nested namespaces from public to private scope
  • Broke downstream consumption where nested namespaces were accessed
  • Broke API documentation which relied on exported namespaces
  • Violated the plugin's philosophy: bundle declarations, not transform semantics

Changes

  • Removed unnecessary duplicateExports() invocation in preProcessNamespaceBody()
  • Removed the else block in fixModifiers() that stripped export from nested namespaces
  • Updated test expectations to preserve export keywords on nested namespaces

Fixes Swatinem#359

Removes two problematic code sections from PR Swatinem#352:

1. Lines 26-28: Unnecessary duplicateExports() call that caused
   superfluous export name duplication in namespace bodies

2. Lines 596-605: Incorrect removal of export keywords from nested
   namespaces, which changed their accessibility from public to private

The plugin should bundle declarations, not transform their semantics.
Nested namespaces with export keywords must remain exported to:
- Maintain public API surface for downstream consumers
- Preserve correct scoping in API documentation
- Keep original type semantics intact
@typhonrt
Copy link

Just in reviewing the code changes via sight reading this indeed solves the problems mentioned. It's great that you listed a general principle of the plugin being "bundle declarations, not transform semantics". I will try your PR fork tomorrow (10/17/25) if this is not merged before then for final real world confirmation. I'm confident though that this PR / change / fix will get your work to a solid functional state. At that point I'm definitely excited to see a new release of rollup-plugin-dts as this is a large improvement in capabilities for namespace support and allows me to do a major release of my own tooling built on top of this plugin.

@typhonrt
Copy link

I have tested this PR in the real world against a complex project and the results are 100% great! Thanks again @privatenumber!

@Swatinem I recommend a merge for this PR and it should finish this round of namespace bundling improvements. This is significant new capabilities, so if possible please consider a new package release; perhaps 6.3.0 semver. Once this is publicly available it will allow me to do a major release of my OSS tooling that uses rollup-plugin-dts, so certainly anticipated.

@Swatinem Swatinem merged commit 883713a into Swatinem:master Nov 19, 2025
8 checks passed
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.

[Code Review] Additional concerns about recent namespace handling changes (not released)

3 participants