search

cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.7k stars 709 forks source link

🚀 Feature Request: enable pages validation for `getPlatformProxy` #5742

Open dario-piotrowicz opened 6 months ago

dario-piotrowicz commented 6 months ago

Describe the solution

getPlatformProxy can be used for both Workers and Pages, but when used for Pages, the wrangler.toml pages-specific validation (see validation-pages) kicks in only if the pages_build_output_dir field is specified. However the field is optional and it would be beneficial to have the pages validation even when such field is not present in the toml file.

The missing validation can cause confusion to users, which might for example end up getting local access to bindings not supported by pages and wondering why such bindings don't work in production (e.g. https://github.com/cloudflare/cloudflare-docs/issues/14186).

I'm proposing to add a flag in the GetPlatformProxyOptions like assumePages/isPages/forcePages/etc... that, once set makes the pages toml validation kick in regardless of the pages_build_output_dir field's presence.

This should help avoiding users' confusion and frustration regarding Pages bindings.

penalosa commented 6 months ago

I'm not sure if we should add this. While pages_build_output_dir is optional in local dev to preserve compatibility with getPlatformProxy, it's not optional if you want to deploy your pages project with a wrangler.toml file—if people want to use a wrangler.toml file for deploying we should be pushing them to add pages_build_output_dir, I think?

dario-piotrowicz commented 6 months ago

No yeah I totally agree with that and I'd imagine that we might want to enforce that all Pages' toml files have the pages_build_output_dir

The fact is that the field is indeed optional for now (and I think that it is kinda optional for deployment too, since if you don't specify it wrangler pages deploy does not fail, but simply ignores the toml file) and the Pages config file support is also in beta, so I am not sure we should force users of getPlatformProxy (which includes frameworks) to adopt it.

TLDR; in the long term I agree that this is not necessary, but until we stabilize, commit and enforce the toml file for Pages this sounds like a pretty good stop-gap? (I think it should also be quite straightforward to implement and subsequently remove)

dario-piotrowicz commented 6 months ago

cc. @petebacondarwin as we chatted about this yesterday and he might want to chime in 🙂