Skip to content
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.

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

refactor(es/helpers): Remove unnecessary exports #9225

Merged
merged 12 commits into from
Jul 13, 2024

Conversation

devjiwonchoi
Copy link
Contributor

@devjiwonchoi devjiwonchoi commented Jul 12, 2024

Description:

Helper methods were exporting twice, _ and it's own function name.
We can map the build script to export _ as it's own name (filename).
Therefore we export only once as _, and map the name on the build script.

// helpers/_foo.js

function _foo() {
  // ...
}

export { _foo as _ }
// index.js

// We know the func name will be `_foo` based on the filename.
export { _ as _foo } from '_foo.js'

Closes #9203

@devjiwonchoi devjiwonchoi requested a review from a team as a code owner July 12, 2024 17:33
@devjiwonchoi devjiwonchoi marked this pull request as draft July 12, 2024 17:33
@devjiwonchoi devjiwonchoi changed the title export as _ refactor: helper methods to export as _ Jul 12, 2024
@devjiwonchoi devjiwonchoi changed the title refactor: helper methods to export as _ refactor: remove unnecessary export of helper methods Jul 12, 2024
@devjiwonchoi devjiwonchoi marked this pull request as ready for review July 12, 2024 18:08
Copy link

codspeed-hq bot commented Jul 12, 2024

CodSpeed Performance Report

Merging #9225 will not alter performance

Comparing devjiwonchoi:jwn/4wbj (4fae2e0) with devjiwonchoi:jwn/4wbj (f3a3141)

Summary

✅ 173 untouched benchmarks

Copy link
Member

@magic-akari magic-akari left a comment

Choose a reason for hiding this comment

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

We also need to update code at packages/helpers/scripts/ast_grep.js, ensuring that the generation logic for CJS is correct.

@devjiwonchoi
Copy link
Contributor Author

Oh, yes! Working on it. 😆

@magic-akari
Copy link
Member

magic-akari commented Jul 13, 2024

There is some code logic that needs to be referred to at https://ast-grep.github.io. If you need help, please let us know.

@magic-akari magic-akari requested a review from kdy1 July 13, 2024 07:30
@kdy1 kdy1 added this to the Planned milestone Jul 13, 2024
@kdy1 kdy1 self-assigned this Jul 13, 2024
kdy1
kdy1 previously approved these changes Jul 13, 2024
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Thank you!


swc-bump:

  • dbg-swc

@kdy1 kdy1 changed the title refactor: remove unnecessary export of helper methods refactor(es/helpers): Remove unnecessary export of helper methods Jul 13, 2024
@kdy1 kdy1 changed the title refactor(es/helpers): Remove unnecessary export of helper methods refactor(es/helpers): Remove unnecessary exports Jul 13, 2024
@kdy1
Copy link
Member

kdy1 commented Jul 13, 2024

I'll merge once @magic-akari approves this PR

@magic-akari
Copy link
Member

There are some helpers from the 'tslib', should they be processed in the same way? @kdy1

@kdy1
Copy link
Member

kdy1 commented Jul 13, 2024

Those are fine. My point was that extra exports are useless, but creates extra entries for some tree shaking thing in turbopack.

@kdy1 kdy1 enabled auto-merge (squash) July 13, 2024 08:09
@kdy1 kdy1 disabled auto-merge July 13, 2024 08:13
@kdy1 kdy1 merged commit 69719c2 into swc-project:main Jul 13, 2024
149 checks passed
@devjiwonchoi devjiwonchoi deleted the jwn/4wbj branch July 13, 2024 08:13
@devjiwonchoi
Copy link
Contributor Author

@magic-akari Thank you for so detailed, kind review!!

kdy1 added a commit to vercel/next.js that referenced this pull request Jul 15, 2024
### What?

Update `@swc/core` and `@swc/helpers`.

### Why?

Updating `@swc/core` is about keeping in sync, and `@swc/helpers` update is about applying swc-project/swc#9225 by @devjiwonchoi, which is an important performance improvement for turbopack tree shaking. (Although tree shaking is not merged yet)

### How?
@kdy1 kdy1 modified the milestones: Planned, v1.7.0 Jul 17, 2024
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 16, 2024
Update `@swc/core` and `@swc/helpers`.

Updating `@swc/core` is about keeping in sync, and `@swc/helpers` update is about applying swc-project/swc#9225 by @devjiwonchoi, which is an important performance improvement for turbopack tree shaking. (Although tree shaking is not merged yet)
@swc-project swc-project locked as resolved and limited conversation to collaborators Aug 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Helper methods do not need to be export functions
3 participants