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

docs: Cloudflare guide update with example #943

Merged
merged 8 commits into from
Nov 13, 2024

Conversation

rmarscher
Copy link
Collaborator

Moving a code example for Cloudflare to Waku so we don't need to reference my public gist and it's easier for everyone to test.

Copy link

vercel bot commented Oct 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
waku ⬜️ Ignored (Inspect) Visit Preview Nov 12, 2024 1:09pm

Copy link

codesandbox-ci bot commented Oct 4, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

thanks for working on this!

check ci errors pls.

Comment on lines 13 to 18
"hono": "^4.6.3",
"react": "19.0.0-rc-2d16326d-20240930",
"react-dom": "19.0.0-rc-2d16326d-20240930",
"react-server-dom-webpack": "19.0.0-rc-2d16326d-20240930",
"waku": "0.21.3",
"wrangler": "^3.80.0"
Copy link
Member

Choose a reason for hiding this comment

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

we pin versions.

Suggested change
"hono": "^4.6.3",
"react": "19.0.0-rc-2d16326d-20240930",
"react-dom": "19.0.0-rc-2d16326d-20240930",
"react-server-dom-webpack": "19.0.0-rc-2d16326d-20240930",
"waku": "0.21.3",
"wrangler": "^3.80.0"
"hono": "4.6.3",
"react": "19.0.0-rc-2d16326d-20240930",
"react-dom": "19.0.0-rc-2d16326d-20240930",
"react-server-dom-webpack": "19.0.0-rc-2d16326d-20240930",
"waku": "0.21.3",
"wrangler": "3.80.0"

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

rename 26_cloudflare to 44_cloudflare and add entries in package.json "scripts".

@rmarscher
Copy link
Collaborator Author

Thank you. I will make these updates after I fix #944. Switching to draft for now.

@rmarscher rmarscher marked this pull request as draft October 6, 2024 03:52
@rmarscher rmarscher force-pushed the docs/cloudflare-example branch 2 times, most recently from 7ab9ab3 to 23f15ee Compare November 6, 2024 05:04
@rmarscher
Copy link
Collaborator Author

I think this should be ready once #989 is merged.

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

LGTM otherwise.

Comment on lines 24 to 26
},
"prettier": {
"singleQuote": true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
},
"prettier": {
"singleQuote": true

Comment on lines 14 to 17
"react": "19.0.0-rc-bf7e210c-20241017",
"react-dom": "19.0.0-rc-bf7e210c-20241017",
"react-server-dom-webpack": "19.0.0-rc-bf7e210c-20241017",
"waku": "0.21.5",
Copy link
Member

Choose a reason for hiding this comment

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

This is updated. Check the main branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is updated. Check the main branch.

OK. All set. I rebased the branch. This is ready for review.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prettier error... one moment...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New error error TS2688: Cannot find type definition file for './.wrangler/types/runtime.d.ts'. 🙄 I'll look for a workaround.

@rmarscher rmarscher force-pushed the docs/cloudflare-example branch 2 times, most recently from 37b4db5 to 214d09a Compare November 10, 2024 14:23
@rmarscher rmarscher force-pushed the docs/cloudflare-example branch from 214d09a to 358b14c Compare November 10, 2024 14:29
@rmarscher rmarscher marked this pull request as ready for review November 10, 2024 14:29
@rmarscher
Copy link
Collaborator Author

It looks like running the example for E2E is hitting a 401 Unauthorized error. I guess running the miniflare dev server communicates with Cloudflare's API and is expecting CLOUDFLARE_API_TOKEN and CLOUDFLARE_ACCOUNT_ID to be set in the environment.

@rmarscher
Copy link
Collaborator Author

I tried a couple workarounds. I need to set up a copy of the CI environment so I can figure out how to workaround the authorization error. I think adding actual CLOUDFLARE_API_TOKEN and CLOUDFLARE_ACCOUNT_ID secrets to the GitHub CI environment would also fix it.

@dai-shi
Copy link
Member

dai-shi commented Nov 12, 2024

I don't think it's a CI issue, but I can't run the test without those envvars, right? Doesn't it need a mock auth server?

@rmarscher
Copy link
Collaborator Author

I think the authorization error that I saw in the CI logs was unrelated. I had an error from updating to the latest minimal client. I'm guessing that it was working locally for me because I had cached vite assets or something like that.

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

LGTM

@dai-shi dai-shi merged commit 33bea3d into wakujs:main Nov 13, 2024
26 checks passed
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