Skip to content

fix: call resolveFileUrl in a deterministic order#10330

Merged
graphite-app[bot] merged 1 commit into
mainfrom
07-17-fix_call_renderfileurl_in_a_deterministic_order
Jul 17, 2026
Merged

fix: call resolveFileUrl in a deterministic order#10330
graphite-app[bot] merged 1 commit into
mainfrom
07-17-fix_call_renderfileurl_in_a_deterministic_order

Conversation

@sapphi-red

@sapphi-red sapphi-red commented Jul 17, 2026 •
edited by hyfdev
Loading

Copy link
Copy Markdown
Member

The resolveFileUrl hook was not called in a deterministic order. This PR traverses chunks and modules in their stable sorted order, so hook invocation no longer depends on module discovery order.

This was referenced Jul 17, 2026
Merged
Merged
Merged
@sapphi-red Graphite App

sapphi-red commented Jul 17, 2026 •
edited
Loading

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.

@sapphi-red sapphi-red mentioned this pull request Jul 17, 2026
Merged
@sapphi-red
sapphi-red force-pushed the 07-17-fix_call_renderfileurl_in_a_deterministic_order branch from 4002120 to 18fba61 Compare July 17, 2026 03:53
@sapphi-red
sapphi-red force-pushed the 07-15-feat_support_import.meta.rolldown_file_url__ branch from 3b64dfc to 09b3d1e Compare July 17, 2026 03:53
@sapphi-red
sapphi-red marked this pull request as ready for review July 17, 2026 04:02
@codspeed-hq

codspeed-hq Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1

Comparing 07-17-fix_call_renderfileurl_in_a_deterministic_order (7ceb442) with 07-15-feat_support_import.meta.rolldown_file_url__ (09b3d1e)

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. ↩

hyfdev
hyfdev requested changes Jul 17, 2026
View reviewed changes

@hyfdev hyfdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single-chunk traversal is now compatible, but the entry-first chunk traversal still reverses Rollup’s resolveFileUrl order for split builds. Stateful hooks therefore receive and assign values to chunks in the opposite order.

Separately, the title and description call the hook renderFileUrl; the public name is resolveFileUrl.

Comment thread crates/rolldown/src/stages/generate_stage/resolve_file_urls.rs
hyfdev
hyfdev approved these changes Jul 17, 2026
View reviewed changes

@hyfdev hyfdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chunk, module, and occurrence traversals are stable, and the regression test shows that the hook no longer depends on module discovery order. Exact cross-chunk Rollup parity is not required, so the manual-chunk difference does not block this change.\n\nThe renderFileUrl name and the Rollup-alignment wording overstate the implementation’s guarantee; those are non-blocking documentation notes.

@hyfdev hyfdev changed the title fix: call renderFileUrl in a deterministic order fix: call resolveFileUrl in a deterministic order Jul 17, 2026
@IWANABETHATGUY Graphite App

IWANABETHATGUY commented Jul 17, 2026 •
edited by graphite-app Bot
Loading

Copy link
Copy Markdown
Member

Merge activity

  • Jul 17, 4:47 AM 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.
  • Jul 17, 6:04 AM UTC: IWANABETHATGUY added this pull request to the Graphite merge queue.
  • Jul 17, 6:12 AM UTC: Merged by the Graphite merge queue.

@sapphi-red
fix: call resolveFileUrl in a deterministic order (#10330)
ce98ae7 
The `resolveFileUrl` hook was not called in a deterministic order. This PR traverses chunks and modules in their stable sorted order, so hook invocation no longer depends on module discovery order.
@graphite-app
graphite-app Bot force-pushed the 07-15-feat_support_import.meta.rolldown_file_url__ branch from 09b3d1e to afac8ae Compare July 17, 2026 06:06
@graphite-app
graphite-app Bot force-pushed the 07-17-fix_call_renderfileurl_in_a_deterministic_order branch from 7ceb442 to ce98ae7 Compare July 17, 2026 06:06
Base automatically changed from 07-15-feat_support_import.meta.rolldown_file_url__ to main July 17, 2026 06:11
@graphite-app
graphite-app Bot merged commit ce98ae7 into main Jul 17, 2026
30 of 31 checks passed
@graphite-app
graphite-app Bot deleted the 07-17-fix_call_renderfileurl_in_a_deterministic_order branch July 17, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hyfdev hyfdev hyfdev approved these changes
@shulaoda shulaoda shulaoda approved these changes
@IWANABETHATGUY IWANABETHATGUY Awaiting requested review from IWANABETHATGUY IWANABETHATGUY is a code owner

Assignees

@IWANABETHATGUY IWANABETHATGUY

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sapphi-red @IWANABETHATGUY @hyfdev @shulaoda