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

update and improve the Cloudflare adapter getting started guide #45

Merged

Conversation

dario-piotrowicz
Copy link
Contributor

@dario-piotrowicz dario-piotrowicz commented Jan 2, 2025

Warning

This PR should be merged only after @opennextjs/[email protected] is released (release PR)


I've updated the getting started guide here mainly for calling out that developers don't necessarily need to create the open-next.config.ts and wrangler.toml files themselves since now that can be done by the Cloudflare adapter.

As I was updating the guide I've found other few spots that I think can be simplified/improved


You should add `.open-next` to your `.gitignore` file to prevent the build output from being committed to your repository.

#### Uninstall `@cloudflare/next-on-pages`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to clean things up a bit here

We have both a Remove '@cloudflare/next-on-pages' (if necessary) and a Uninstall '@cloudflare/next-on-pages' section, this seems pretty confusing to me, also the hierarchy is a bit out of whack I think (for example, Add '.open-next' to '.gitignore' is under Remove '@cloudflare/next-on-pages' (if necessary), that doesn't seem correct to me)


```diff
"scripts": {
- "pages:build": "npx @cloudflare/next-on-pages",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are the scripts the C3 generates for users, but people can create their apps manually and not via C3 or update the name of such scripts, so I think that maybe it makes sense to just be generic here and say to remove any reference to the next-on-pages package and be done with it

@@ -23,43 +23,66 @@ Use the template from `flarelabs-net/workers-next` as indicated above to use 0.3
First, install [@opennextjs/cloudflare](https://www.npmjs.com/package/@opennextjs/cloudflare):

```sh
npm install --save-dev @opennextjs/cloudflare
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added @latest to be consistent with wrangler below (I'm just making this change for consistency, I don't have a strong preference here, we can or not have @latest, whatever you prefer)


Install the [Wrangler CLI](https://developers.cloudflare.com/workers/wrangler/) as a devDependency:

```npm
npm install -D wrangler@latest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated -D to --save-dev to be consistent with @opennextjs/cloudflare above (I'm just making this change for consistency, I don't have a strong preference here, we can go with either version)

@dario-piotrowicz dario-piotrowicz force-pushed the dario/cloudflare-guide-update branch 4 times, most recently from d94738f to 8a36931 Compare January 2, 2025 17:14
@dario-piotrowicz dario-piotrowicz force-pushed the dario/cloudflare-guide-update branch from 8a36931 to 96b771d Compare January 2, 2025 17:15
@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review January 7, 2025 10:33
@dario-piotrowicz dario-piotrowicz requested a review from vicb January 7, 2025 10:33
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Thanks!

@vicb vicb merged commit 2beac6f into opennextjs:main Jan 7, 2025
1 check passed
@dario-piotrowicz dario-piotrowicz deleted the dario/cloudflare-guide-update branch January 13, 2025 10:41
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.

2 participants