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

feat: add cloudflare-pages preset #210

Merged
merged 22 commits into from
Aug 24, 2022
Merged

feat: add cloudflare-pages preset #210

merged 22 commits into from
Aug 24, 2022

Conversation

DaniFoldi
Copy link
Contributor

@DaniFoldi DaniFoldi commented May 7, 2022

πŸ”— Linked issue

#196

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 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)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Related to discussions #110 #196, a few in nuxt/framework

This PR intends to add a preset for Cloudflare Pages Functions - which is currently in Open Beta. Docs can be found at https://developers.cloudflare.com/pages/platform/functions/ .

Pages Functions are based on the modules worker type, so a different deployment preset will be needed.

update since first drafted, removed no longer relevant beta warnings

Functions are still in beta, however Direct Upload is now available for Pages, and I fixed all the issues I could find.

update again

Docs have now been updated (feedback welcome), the newest version of std-env should allow for automatic detection (only via git integration).

Have a good day.

πŸ“ Checklist

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

@tuarrep
Copy link

tuarrep commented Aug 12, 2022

Hey @DaniFoldi πŸ‘‹

Do you need help to test / finish this one?

I'm currently investigating to migrate to Cloudflare pages and I require this preset.

@DaniFoldi
Copy link
Contributor Author

Hi!

I started writing the docs for the preset earlier today, so it should be ready today or tomorrow - the one remaining thing after this PR will be figuring out how to give api routes the request context.

@codecov
Copy link

codecov bot commented Aug 13, 2022

Codecov Report

Merging #210 (abe5fd5) into main (5356f62) will increase coverage by 0.08%.
The diff coverage is 42.30%.

@@            Coverage Diff             @@
##             main     #210      +/-   ##
==========================================
+ Coverage   53.48%   53.57%   +0.08%     
==========================================
  Files          54       54              
  Lines        3468     3494      +26     
  Branches      368      368              
==========================================
+ Hits         1855     1872      +17     
- Misses       1258     1267       +9     
  Partials      355      355              
Impacted Files Coverage Ξ”
src/utils/index.ts 23.52% <0.00%> (-0.82%) ⬇️
src/presets/cloudflare.ts 63.63% <44.00%> (+11.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@DaniFoldi
Copy link
Contributor Author

@tuarrep I believe it's ready for testing, and feel free to leave any feedback on the docs as well.
(vercel had a problem deploying, but doesn't tell what the problem was, so if it's something that I messed up (probably) could someone with access let me know?

@DaniFoldi DaniFoldi marked this pull request as ready for review August 13, 2022 09:38
maybe later we split but currently they are categorized per vendor

const r = await nitroApp.localCall({
env,
context,
Copy link
Member

@pi0 pi0 Aug 24, 2022

Choose a reason for hiding this comment

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

FYI this currently has no effect (src). unenv only picks known keys from input (url, method, headers, host, protocol, body). But we should definitely add cf context in later changes.

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 for this nice work πŸ’― I guess we might have problems with public directory handling (.output/public) should be picked. Nuxt used top level dist/ which is probably if you tested and working. We could do the same btw with preset. Will have to try on nitro-edge.

@pi0 pi0 merged commit ed7d0db into unjs:main Aug 24, 2022
@ricky11
Copy link

ricky11 commented Sep 9, 2022

does this mean by adding ./server/api/function.js and with git integration the preset will be auto set as cloudflare_pages, and the /function directory will be created with the function.js api? I have tested it here, and while the function folder is created, no function is actually deployed on cloudflare network . Tried running CF_PAGES=1 npm run generate in this stackblitz terminal https://stackblitz.com/edit/github-dxg5qg?file=server%2Fapi%2Fcustomer.get.js

Updated : Changed cloudflare build settings to npm run build , this does seem to pick up the /server/api/[routes] and inject them into /functions/[[path]].js However, for some reasons only GET requests are working, POST requests are getting 405 Method not allowed -- anyone else have better results?

WinterYukky pushed a commit to WinterYukky/nitro that referenced this pull request Nov 1, 2022
@Plinpod
Copy link

Plinpod commented Jan 3, 2023

Seeing same issue with non-get requests returning 405

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.

None yet

7 participants