Skip to content

refactor: use the traced file copy from OpenNext #282

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

Merged
merged 2 commits into from
Jan 27, 2025
Merged

refactor: use the traced file copy from OpenNext #282

merged 2 commits into from
Jan 27, 2025

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Jan 25, 2025

fixes #251
relates to #14

Before this PR, the Next compiled output was copied to .open-next/.next and some files were used/patched from there.

This PR makes sure we always use the files from .open-next/.server-functions/<fnName> instead.

This allows to simplify a lot of the config object that was used before.

@vicb vicb requested a review from dario-piotrowicz January 25, 2025 12:43
Copy link

changeset-bot bot commented Jan 25, 2025

🦋 Changeset detected

Latest commit: 19f4c2b

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

Copy link

pkg-pr-new bot commented Jan 25, 2025

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/cloudflare@282

commit: 19f4c2b

@vicb vicb requested a review from james-elicx January 25, 2025 12:53
Copy link
Collaborator

@james-elicx james-elicx left a comment

Choose a reason for hiding this comment

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

Nice, looks good

@vicb
Copy link
Contributor Author

vicb commented Jan 27, 2025

Thanks for the review @james-elicx, I'll wait for @dario-piotrowicz to review before I merge this.

@vicb
Copy link
Contributor Author

vicb commented Jan 27, 2025

Thanks for the review @james-elicx, I'll wait for @dario-piotrowicz to review before I merge this.

Oops, I merged @james-elicx PR into this one (I forgot it was based on this branch).

@dario-piotrowicz you can review the first commit only, thanks!

Copy link
Contributor

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

Looks very promising! 😄 🥳

I'm halfway through my review, just leaving a partial review 🙂


Nit: I didn't comment on them since there are quite a few, but you're introducing a bunch of joins that are inconsistent with your convention

@vicb vicb force-pushed the paths branch 2 times, most recently from cf08a5c to 91625b2 Compare January 27, 2025 13:05
@vicb
Copy link
Contributor Author

vicb commented Jan 27, 2025

@dario-piotrowicz I integrated your first batch of feedback, thanks!

Copy link
Contributor

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

Looks great to me! 😄 such a nice improvement! 🚀

@vicb
Copy link
Contributor Author

vicb commented Jan 27, 2025

Thanks for the reviews @james-elicx and @dario-piotrowicz !

@vicb vicb merged commit 4ec334a into main Jan 27, 2025
7 checks passed
@vicb vicb deleted the paths branch January 27, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: drop projectOpts
3 participants