Skip to content

Clarified limitations of using Cloudflare adapter with Node.js#7414

Merged
sarah11918 merged 7 commits into
withastro:mainfrom
jmho:added-node-cloudflare-details
Mar 17, 2024
Merged

Clarified limitations of using Cloudflare adapter with Node.js#7414
sarah11918 merged 7 commits into
withastro:mainfrom
jmho:added-node-cloudflare-details

Conversation

@jmho
Copy link
Copy Markdown
Contributor

@jmho jmho commented Mar 15, 2024

Description (required)

Added additional clarification about the limitations of deploying with the Cloudflare adapter when using packages that import the Node.js runtime without using the import * from node* syntax. The error message that appears when you run into this is unclear especially in regards to why it is happening, and what the next steps should be for someone.

My hope is that with updates to the docs, a developer can understand the following: This is not a bug within Astro, why this error may sometimes occur, and what the next steps are if someone is developing an Astro app that imports relies on packages that might not conform to the import * from node* syntax.

Please let me know if anything needs to be clarified or updated!

(Thanks to @alexanderniebuhr for helping me figure out the original issue!)

Discord: hgswiz

Related issues & labels (optional)

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 15, 2024

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

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Mar 17, 2024 5:23pm

@astrobot-houston
Copy link
Copy Markdown
Contributor

Hello! Thank you for opening your first PR to Astro’s Docs! 🎉

Here’s what will happen next:

  1. Our GitHub bots will run to check your changes.
    If they spot any broken links you will see some error messages on this PR.
    Don’t hesitate to ask any questions if you’re not sure what these mean!

  2. In a few minutes, you’ll be able to see a preview of your changes on Vercel 🥳

  3. One or more of our maintainers will take a look and may ask you to make changes.
    We try to be responsive, but don’t worry if this takes a few days.

Copy link
Copy Markdown
Member

@alexanderniebuhr alexanderniebuhr left a comment

Choose a reason for hiding this comment

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

@jmho Thank you for updating the docs.. I'll added some comments already, and I'll also make sure our docs team reviews this too.

Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
@sarah11918
Copy link
Copy Markdown
Member

Just a note that I will come in and do a docs (text/language/formatting) review once all of the technical issues and questions have been resolved!

I am not the subject matter expert here, so I am trusting @alexanderniebuhr to clarify all the necessary/important details, and will wait for him to be 100% certain about the content. Then I'll just make sure it sounds pretty and fits with docs structure/standards! 🙌

Updated link to use header

Co-authored-by: Alexander Niebuhr <alexander@nbhr.io>
Copy link
Copy Markdown
Contributor Author

@jmho jmho 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 the review! I added some additional clarification and accepted some of your changes.

Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
alexanderniebuhr and others added 2 commits March 17, 2024 16:55
Co-authored-by: Justin Ho <59701887+jmho@users.noreply.github.com>
Co-authored-by: Justin Ho <59701887+jmho@users.noreply.github.com>
Copy link
Copy Markdown
Member

@alexanderniebuhr alexanderniebuhr left a comment

Choose a reason for hiding this comment

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

I approve Cloudflare specific technical aspects. Now waiting for docs review!

cc @sarah11918

Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
Comment thread src/content/docs/en/guides/deploy/cloudflare.mdx Outdated
@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! labels Mar 17, 2024
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, @jmho! We appreciate the extra guidance, and are excited to welcome you to Team Docs! 🥳

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, @jmho! We appreciate the extra guidance, and are excited to welcome you to Team Docs! 🥳

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

Labels

add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Astro with cloudflare adapter fails to build when using firebase

4 participants