Skip to content

fix(generate): keep JSON namespace interface under preserveModules#10028

Merged
shulaoda merged 2 commits into
graphite-base/10028from
fix-json-namespace-preserve-modules
Jun 30, 2026
Merged

fix(generate): keep JSON namespace interface under preserveModules#10028
shulaoda merged 2 commits into
graphite-base/10028from
fix-json-namespace-preserve-modules

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented Jun 30, 2026

Copy link
Copy Markdown
Member

What

Builds on the parent PR (#10020). A JSON module imported as a namespace (import * as ns from './x.json') keeps every top-level key materialized — the finalizer only inlines JSON keys when the module namespace object is not included (need_inline_json_prop, see finalizer_context.rs).

This is pre-existing behavior, not a regression. On main a namespace-imported JSON chunk under preserveModules already exports its full interface correctly — import('./dist/data.js') exposes every top-level key. Unlike #10020 (which is a #9934 regression), there is no namespace bug on main. I verified the guard test passes on main.

The reason this PR exists is purely to keep the stack consistent: the parent PR's #10020 narrowing (skip keys absent from json_module_none_self_reference_included_symbol) is too coarse for the namespace case — those keys are absent from that set yet still materialized, so the parent PR alone would drop them:

// parent PR (#10020) alone: only the namespace object survives
export { data_exports };
// with this PR: pre-existing full interface preserved
export { answer, data_exports, data_default as default, nested };

Fix

Mirror the finalizer's full inlining condition rather than the key-set alone: only drop a key when JSON property inlining is actually active — JSON, ESM exports, and the namespace object not included. The stack's net behavior then matches main for the namespace case.

Test

crates/rolldown/tests/rolldown/misc/preserve_modules/json_namespace_preserves_exports: import * as a JSON module under preserveModules and assert (via _test.mjs) that import('./dist/data.js') re-exposes every top-level key plus default. Verified: passes on main, fails on the parent PR alone, passes here.


Stacked on the #10020 fix.

…ce import

A JSON module imported as a namespace (`import * as ns`) keeps every top-level
key materialized — the finalizer only inlines keys when the module namespace
object is NOT included (`need_inline_json_prop`). The #10020 filter, which
dropped any key absent from `json_module_none_self_reference_included_symbol`,
also removed those still-materialized keys from the preserved chunk's exports,
so `import("./dist/data.js")` no longer exposed them.

Mirror the finalizer's full inlining condition: only drop a key when JSON
property inlining is actually active (ESM exports, and the namespace object not
included). A namespace-imported JSON chunk now re-exports its complete
interface.

IWANABETHATGUY commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@IWANABETHATGUY IWANABETHATGUY changed the title fix(generate): keep full JSON interface under preserveModules namespace import fix(generate): keep JSON namespace interface under preserveModules Jun 30, 2026
@graphite-app graphite-app Bot changed the base branch from fix-10020-json-preserve-modules-default to graphite-base/10028 June 30, 2026 10:00
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review June 30, 2026 13:05
@codspeed-hq

codspeed-hq Bot commented Jun 30, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix-json-namespace-preserve-modules (642d90b) with main (5f71cbf)2

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on graphite-base/10028 (8c87f8d) during the generation of this report, so main (5f71cbf) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

shulaoda commented Jun 30, 2026

Copy link
Copy Markdown
Member

Merge activity

  • Jun 30, 8:22 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 30, 8:22 PM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • Jun 30, 8:22 PM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..
  • Jun 30, 8:22 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 30, 8:38 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 30, 8:39 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 30, 8:40 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 30, 8:41 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 30, 8:41 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.

@shulaoda shulaoda merged commit 556e8e6 into graphite-base/10028 Jun 30, 2026
40 of 42 checks passed
@shulaoda shulaoda deleted the fix-json-namespace-preserve-modules branch June 30, 2026 20:38
@shulaoda shulaoda restored the fix-json-namespace-preserve-modules branch June 30, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants