Skip to content

fix: module library export generation for reexport #19459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Apr 24, 2025
Merged

fix: module library export generation for reexport #19459

merged 3 commits into from
Apr 24, 2025

Conversation

alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented Apr 23, 2025 •
edited
Loading

What kind of change does this PR introduce?

bugfix - #19386 (comment)

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

No

/cc @JSerFeng

Looks like we still need generate var/const extra variable for reexport exports, otherwise we can faced with the same top level name in scope

@alexander-akait alexander-akait mentioned this pull request Apr 23, 2025
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 23, 2025 •
edited
Loading

Benchmarks:

  • "devtool-source-map": HEAD (c0a8664) 651 ms ± 47 ms [640 ms; 662 ms] (51 runs)
  • "devtool-source-map": BASE (519187b) 637 ms ± 38 ms [628 ms; 646 ms] (52 runs)
  • "devtool-source-map" change: HEAD (651 ms ± 47 ms [640 ms; 662 ms]) is -2% the same as BASE (BASE) (637 ms ± 38 ms [628 ms; 646 ms])
  • "many-chunks": HEAD (c0a8664) 1088 ms ± 35 ms [1077 ms; 1098 ms] (32 runs)
  • "many-chunks": BASE (519187b) 1074 ms ± 29 ms [1065 ms; 1082 ms] (32 runs)
  • "many-chunks" change: HEAD (1088 ms ± 35 ms [1077 ms; 1098 ms]) is -1% the same as BASE (BASE) (1074 ms ± 29 ms [1065 ms; 1082 ms])
  • "many-modules": HEAD (c0a8664) 584 ms ± 24 ms [579 ms; 590 ms] (55 runs)
  • "many-modules": BASE (519187b) 588 ms ± 28 ms [581 ms; 594 ms] (55 runs)
  • "many-modules" change: HEAD (584 ms ± 24 ms [579 ms; 590 ms]) is 1% the same as BASE (BASE) (588 ms ± 28 ms [581 ms; 594 ms])
  • "popular-libraries": HEAD (c0a8664) 1470 ms ± 31 ms [1460 ms; 1481 ms] (25 runs)
  • "popular-libraries": BASE (519187b) 1522 ms ± 47 ms [1505 ms; 1538 ms] (24 runs)
  • "popular-libraries" change: HEAD (1470 ms ± 31 ms [1460 ms; 1481 ms]) is 4% faster than BASE (BASE) (1522 ms ± 47 ms [1505 ms; 1538 ms])

@JSerFeng
Copy link
Contributor

JSerFeng commented Apr 24, 2025 •
edited
Loading

Thanks! LGTM, and we need to align this in Rspack as well

@alexander-akait alexander-akait merged commit 648e026 into main Apr 24, 2025
48 of 58 checks passed
@alexander-akait alexander-akait deleted the issue-19386 branch April 24, 2025 15:51
@alexander-akait
Copy link
Member Author

@JSerFeng Improved it by checking top level

3ru pushed a commit to 3ru/webpack that referenced this pull request Jul 1, 2025
@alexander-akait @3ru
fix: module library export generation for reexport (webpack#19459)
a65db8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alexander-akait @JSerFeng