cloudflare / pages-plugins

MIT License
57 stars 20 forks source link

Fix onRequestGet to target only forms with data-static-form-name #41

Closed savtrip closed 8 months ago

savtrip commented 10 months ago

Hi @GregBrimble, this closes issue #39

As stated in the docs: The Static Forms Pages Plugin intercepts all form submissions made which have the data-static-form-name attribute set. This allows you to take action on these form submissions by, for example, saving the submission to KV.

Source: https://developers.cloudflare.com/pages/functions/plugins/static-forms/

However, in reality, it targeted every single form. I have another form on my website and noticed the action was been removed which lead me to report this on the Cloudflare forms: https://community.cloudflare.com/t/cloudflare-strips-external-domains-from-form-action-attribute/595529/5

I put an if statement in to check if that attribute exists on the form which avoids the modification of any other forms on the website. I also removed the line that removes the form "action". In my opinion that can be documented clearly in the plugin e.g. "don't add an action in otherwise it may lead to unexpected behaviour". It can of course stay in, I just thought it would be better to leave the decision to the programmer to leave it out or add it in if they have some unusual use case.

Thanks for considering this pull request, I appreciate Cloudflare open sourcing this stuff unlike other static site generator platforms. Thanks Greg.

changeset-bot[bot] commented 10 months ago

🦋 Changeset detected

Latest commit: cf278ce51e9b88907e9d2637742f800a4fa5d224

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------------------------------- | ----- | | @cloudflare/pages-plugin-static-forms | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

GregBrimble commented 8 months ago

Thanks for your contribution! I made three small changes:

Thanks again!

savtrip commented 8 months ago

@GregBrimble Thanks for merging this in, much appreciated.