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): add local dev section #1772

Merged
merged 4 commits into from
Oct 3, 2023
Merged

docs(cloudflare): add local dev section #1772

merged 4 commits into from
Oct 3, 2023

Conversation

McPizza0
Copy link
Contributor

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Added a build step to wrangler.toml example

Previously:
Any code changes would require a new nitropack build followed by restarting wrangler dev

Now:
wrangler watches the routes dir and nitro.config.ts. On change, wrangler dev rebuilds nitro Local dev experience when using wrangler dev now matches nitropack dev

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

Added the build step to wrangler.toml example

Previously:
Any code changes would require a new `nitropack build` followed by restarting `wrangler dev`

Now:
`wrangler` watches the routes dir and `nitro.config.ts`. On change, `wrangler` dev rebuilds `nitro`
Local dev experience when using `wrangler dev` now matches `nitropack dev`
@nuxt-studio
Copy link
Contributor

nuxt-studio bot commented Sep 29, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
nitro Edit on Studio β†—οΈŽ View Live Preview 5f5c39d

@McPizza0 McPizza0 changed the title DOCKS: Update cloudflare: ++ watch/build step to wrangler example DOCS: Update cloudflare: ++ watch/build step to wrangler example Sep 29, 2023
@McPizza0 McPizza0 marked this pull request as draft September 29, 2023 14:13
@pi0 pi0 changed the title DOCS: Update cloudflare: ++ watch/build step to wrangler example docs(cloudflare): update wrangler.toml to add build-step Oct 2, 2023
@pi0
Copy link
Member

pi0 commented Oct 2, 2023

Hi and thanks for this PR. Normally , we expect always a build step happens before running platform-specific deployment steps (such as wrangler dev) + we are considering to support cloudflare (and other presets) in dev/watch mode.

For this to move forward, i would suggest a new section to the end about adding build steps.

@McPizza0
Copy link
Contributor Author

McPizza0 commented Oct 3, 2023

@pi0
wrangler.toml supports different settings for different environments - as per: https://developers.cloudflare.com/workers/wrangler/configuration/#environments

We could change the toml build step to be

[env.development.build]
command = "NITRO_PRESET=cloudflare npm run build"
cwd = "./"
watch_dir = ["./routes", "./nitro.config.ts"]

This would only trigger when running
wrangler dev --env development

Leaving the rest of the steps as it was before

@pi0
Copy link
Member

pi0 commented Oct 3, 2023

i see but we ideally don’t need to run full build on each dev change. That’s why asking to add it as new advanced section until improvements this area.

@McPizza0
Copy link
Contributor Author

McPizza0 commented Oct 3, 2023

100%
Thats fair
have moved instructions to advanced section @pi0

@McPizza0 McPizza0 marked this pull request as ready for review October 3, 2023 08:59
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thanks!

@pi0 pi0 changed the title docs(cloudflare): update wrangler.toml to add build-step docs(cloudflare): add local dev section Oct 3, 2023
@pi0 pi0 merged commit 2606a83 into unjs:main Oct 3, 2023
@McPizza0 McPizza0 deleted the patch-1 branch October 3, 2023 17:29
@pi0 pi0 mentioned this pull request Oct 5, 2023
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