Skip to content

perf: drop the babel dependency #843

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 1 commit into from
Aug 4, 2025
Merged

perf: drop the babel dependency #843

merged 1 commit into from
Aug 4, 2025

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Aug 4, 2025

The former patch of next server is not enough.

The code was changed sometimes in Next 15.

We should upstream that to aws.

Before (e2e/experimental)

image

After

image

/cc @conico974 @sommeeeer

@vicb vicb requested review from conico974 and sommeeeer August 4, 2025 20:19
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 4, 2025

🦋 Changeset detected

Latest commit: 44194a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@opennextjs/cloudflare Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Aug 4, 2025

Open in StackBlitz

npm i https://pkg.pr.new/@opennextjs/cloudflare@843

commit: 44194a9

sommeeeer
sommeeeer approved these changes Aug 4, 2025
View reviewed changes
Copy link
Collaborator

@sommeeeer sommeeeer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, does this get rid of that pesky switch case warning? Should you only apply it on Next >= 15.4?

@vicb
Copy link
Contributor Author

vicb commented Aug 4, 2025

LGTM, does this get rid of that pesky switch case warning?

Yes, that warning is removed.

Should you only apply it on Next >= 15.4?

I think somebody reported it for 15.3.<I don't remember what> last week

@vicb
Copy link
Contributor Author

vicb commented Aug 4, 2025

Should you only apply it on Next >= 15.4?

I think somebody reported it for 15.3.<I don't remember what> last week

actually it was 15.2.5

@vicb
Copy link
Contributor Author

vicb commented Aug 4, 2025

Oh and thanks for another review 🙏

@vicb vicb merged commit 10fc603 into main Aug 4, 2025
7 checks passed
@vicb vicb deleted the vicb/babel-again branch August 4, 2025 20:45
@workers-frameworks workers-frameworks mentioned this pull request Aug 4, 2025
Merged
conico974
conico974 approved these changes Aug 4, 2025
View reviewed changes
Copy link
Collaborator

@conico974 conico974 left a comment

Choose a reason for hiding this comment

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

LGTM! And yeah we should add it to aws as well

@conico974
Copy link
Collaborator

You guys are too fast 😄

This was referenced Aug 4, 2025
Closed
Closed
@LubomirGeorgiev
Copy link

LubomirGeorgiev commented Aug 5, 2025 •
edited
Loading

@vicb

This helped a lot. It significantly reduced the worker size.

Before:
Total Upload: 25229.97 KiB / gzip: 6136.94 KiB

After:
Total Upload: 22279.11 KiB / gzip: 5475.65 KiB

Do you have in mind other similar optimizations like this one?

vicb added a commit that referenced this pull request Sep 1, 2025
@vicb
Use patchNodeEnvironment from aws
aa693b6 
`patchNodeEnvironment` was introduced in #843 to drop the babel dependency.
It was upstream into aws via opennextjs/opennextjs-aws#948.
This PR drops the local version to use the upstream version.
@vicb vicb mentioned this pull request Sep 1, 2025
Merged
vicb added a commit that referenced this pull request Sep 1, 2025
@vicb
Use patchNodeEnvironment from aws (#863)
cc1ecbd 
`patchNodeEnvironment` was introduced in #843 to drop the babel dependency.
It was upstream into aws via opennextjs/opennextjs-aws#948.
This PR drops the local version to use the upstream version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@conico974 conico974 conico974 approved these changes

@sommeeeer sommeeeer sommeeeer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vicb @conico974 @LubomirGeorgiev @sommeeeer