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 Outdated Hono Docs for Pages #15786

Closed
wants to merge 1 commit into from

Conversation

NicoPlyley
Copy link

@NicoPlyley NicoPlyley commented Jul 24, 2024

Summary

I updated the documentation for using Hono on Cloudflare Pages. The current documentation is outdated since Hono (for CF Pages) now uses Vite. C3 is also made for Hono on CF Workers and requires a lot of manual setup on the user's part to get it working with Pages, which is why create-hono is used here.

Discussions about this has also been made here: honojs/hono#838

Please review @yusukebe

Documentation checklist

  • The documentation style guide has been adhered to.
  • If a larger change - such as adding a new page- an issue has been opened in relation to any incorrect or out of date information that this PR fixes.
  • Files which have changed name or location have been allocated redirects.

Closes #15785

@dario-piotrowicz
Copy link
Member

Hi @NicoPlyley thanks so much for the PR 🙂 🙏

We've discussed it internally and we agree that we ideally want to recommend the use of the use of the create cloudflare CLI (C3).

Your PR is instead suggesting the direct use to the create hono CLI which is not really the direction we want to take 😢

We've discussed this and instead of updating the hono guide in the way suggested in the PR we'd want to:

  • update the hono entry under Website or web app (to use the hono pages template)
  • add a new API hono entry (to use the hono workers template)
  • update this guide accordingly (with the web app/pages case, since this guide is pages specific)

would you like to help in the above and update this PR? If so I would love to help if you want/need 🙂

otherwise you can leave this with us and we'll take care of it, whatever you prefer 🙂

(in any case, thank you so very much again for the PR! it's truly appreciated! sorry for the pushback 🥲)

@NicoPlyley
Copy link
Author

Hi @dario-piotrowicz,

I completely understand, I was following the recommendation of @yusukebe which you can see our discussion of using C3 vs create Hono in the issue I linked above.

Either way I'd be happy to help, it sounds like your recommend adding another template to C3 for using Hono on Pages, is that correct? And then update this existing PR accordingly?

Please let me know if I'm understanding this right and I will submit a PR accordingly!

@yusukebe
Copy link
Contributor

yusukebe commented Jul 26, 2024

Hi @NicoPlyley

Thank you for the PR! As @dario-piotrowicz said, we've discussed it internally and have some plans to improve the C3. So, sorry for changing what I said the first time in the discussion.

@dario-piotrowicz may be able to show the details later, but in our current plan, C3 will have an Hono section in the same layer as Website or web app, and you can choose a template from both Workers and Pages templates. (Maybe it's not correct, my misunderstanding. Wait for a correct explanation) This will be great for Hono!

I think this is good except for the create-hono part. I like the simplicity of it!

@dario-piotrowicz
Copy link
Member

Hi @dario-piotrowicz,

I completely understand, I was following the recommendation of @yusukebe which you can see our discussion of using C3 vs create Hono in the issue I linked above.

Hey @NicoPlyley thanks a lot for your understanding, sorry I did check the issue you shared, I am sorry about all the back and forth regarding this! 🙇 I really appreciate your patience and willingness to update things ❤️ 🙏

Either way I'd be happy to help, it sounds like your recommend adding another template to C3 for using Hono on Pages, is that correct? And then update this existing PR accordingly?

What we want to end up with in C3 is something like the following:

* "Hello World" Worker
* "Hello World" Worker (Python)
* Website or web app
  * Analog
  * Astro
  * ...
  * Hono => Hono's Pages template (*1)
* Hono API Worker (name TBD)=> Hono's Workers template (*2)
* ...

So this means that we need to:

  • update the C3 hono Website or web app entry (*1) here
  • update this PR/guide accordingly
  • afterwords (or in parallel, or whatever we prefer) we can add the new Hono API Worker entry (*2) (this won't be part of the PR/guide)

Please let me know if I'm understanding this right and I will submit a PR accordingly!

hopefully my message clarify things, we don't want to add anther hono on pages template, but update the current hono on pages one and add a hono on workers one 🙂

Thanks so very much again! if there's anything I can help with of clarify I'm always here to help 😄

@NicoPlyley
Copy link
Author

So, I've looked into this a bit more, and to my understanding, @dario-piotrowicz, snippets cannot be added to the file because the loadSnippets function in codemod only supports .ts and .js extensions, where Hono is using .tsx.

The only thing the snippets add is the Bindings, which are auto-generated. So, I suggest just adding this to the Hono template directly or not adding the snippets at all. Let me know your thoughts, @yusukebe.

@yusukebe
Copy link
Contributor

Hi @NicoPlyley !

Ideally, for Hono's Pages template, C3 would also update wrangler.toml, generate a type definition file, and add the cf-typegen command to package.json. We can add the same things to the template on the Hono side, but if the workers-sdk side makes changes to C3, it will not be able to keep up with them.

@dario-piotrowicz (and @jculvey ?)

If we add Hono's Pages template to C3, the current C3 implementation doesn't seem to be able to update wrangler.toml or add the typegen command, for that template. We have to discuss it somewhere!

@NicoPlyley
Copy link
Author

NicoPlyley commented Jul 27, 2024

@yusukebe I want to clarify we can still update/add the wrangler.toml and add the cf-typegen command as that is separate from how snippets work. Just adding the following in the index.tsx:

type Bindings = {
  [key in keyof CloudflareBindings]: CloudflareBindings[key]
}

const app = new Hono<{ Bindings: Bindings }>()

Is not possible since loadSnippets does not appear to allow .jsx extensions, at least that's how I am understanding it.

This is currently what is being added, which is set up for using Workers.

@dario-piotrowicz
Copy link
Member

@NicoPlyley Sorry for the late reply! 🙇

Do you need to use loadSnippets? could the file not be included as a template?
For example like so:
https://github.com/cloudflare/workers-sdk/blob/c3e19b790bb597b78e0109a162ca8049b5eaf973/packages/create-cloudflare/templates/hono/c3.ts#L62-L64

You could also have different variants (for example js and ts) as you can see here for example: https://github.com/cloudflare/workers-sdk/blob/c3e19b790bb597b78e0109a162ca8049b5eaf973/packages/create-cloudflare/templates/next/c3.ts#L182-L212

Would this not work for Hono?

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Aug 1, 2024

Also since we discussed this issue the C3 UI has actually been updated:
Screenshot 2024-08-01 at 22 52 45

So I am guessing that now we would want the Pages hono app to be under Framework Starter and the Api hono one to be under Demo application... I believe... I'd be keen to see what @edmundhung thinks (as he's the developer that made the above UI change: cloudflare/workers-sdk#6320)

@KimJ15
Copy link
Collaborator

KimJ15 commented Sep 27, 2024

👋Hi there, we’re going to close this PR. All PRs prior to August 12 need to be reworked due to the platform migration to Astro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hono on Cloudflare Pages is out of date
7 participants