Skip to content

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