-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[gatsby-wordpress-source] Adds initial support for Gravity Forms #5599
Conversation
Deploy preview for gatsbygram ready! Built with commit 9a7c731 |
Deploy preview for using-drupal ready! Built with commit 9a7c731 |
Deploy preview for gatsbygram ready! Built with commit c2af7a9 |
Deploy preview for using-drupal ready! Built with commit c2af7a9 |
_hostingWPCOM, | ||
_auth, | ||
_accessToken, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we would want to parallelize fetches because right now this would get 1 form data at a time which will slow bootstraping process if there are multiple forms.
Take a look how we pararelize requests in https://github.com/kennethormandy/gatsby/blob/830da2ff03729acaa2267c7cec4110ffd5aeed61/packages/gatsby-source-wordpress/src/fetch.js#L374-L381
(I see that you copied logic from menus fetching - that could also benefit from same treatment if possible)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pieh Thanks for the quick review! I’ve got an attempt at it here, but frankly I don’t think I know async
/await
well enough to finish it just yet. e1fbf74
I’m also not sure if this approach is totally necessary though, is this outcome that you are looking for? That each property is requested via a URL rather than just doing one request?
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/title
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/description
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/labelPlacement
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/descriptionPlacement
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/button
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/fields
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/version
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/id
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/useCurrentUserAsAuthor
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/postContentTemplateEnabled
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/postTitleTemplateEnabled
=== [ Fetching wordpress__gf_forms ] === http://localhost:8080/wp-json/gf/v2/forms/1/postTitleTemplate
…rather than now, that does just one request to /forms/1
and then gets all that info. This might be a side effect of me doing something else wrong though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant doing parallel requests for forms (not for individual properties of single form)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, that’s what I was aiming to do, but I think I overcomplicated the async
/await
in an array bit.
…ugin tutorial" This reverts commit 3d5aa11.
Oops, committed to the wrong PR 😅 |
I was just thinking - is this something you can implement in actual wordpress plugin? Add all required fields to |
@pieh I don’t so, not without adding a custom plugin alongside Gravity Forms to manually add that endpoint. My goal was to have just work with the plugin in the same way ACF does.
The Gravity Forms v2 API is repo is visible right now, so I can open an issue there, but I think it’s mainly keeping the existing API while changing a few things to fit the WP REST API conventions better (although perhaps this falls within that). |
Well ACF does require separate wordpress plugin that exposes fields to REST API, so this plugin I propose would serve same purpose.
But gravity plugin itself is premium plugin, correct? None of maintainers have this plugin, so we can't properly support any future bug reports :/ That's why I push for doing it on wordpress side of things. There is also another approach - but this isn't something that will be done soon I think: I see 2 good candidates for APIs:
It was already attempted for normalization part ( #3783 ), but solution used for prioritization wasn't really that great, but concept of subplugins prove to be working in remark, so I'm sure we can make it work for wordpress. |
Good point, I forgot about that.
Yes, it is. Fair enough. I’d be open to helping with this, because I’ve done it for myself anyway (I intend to continue to contributing to Gatsby), but I can totally understand why you’d say the responsibility for that should live on the WordPress side of things.
The above all sounds great to me. I would be happy to port this as a test case, but I get what you’re saying about it not being done soon. Is there something in the short term I could help with there? I’m just using a fork with only these changes in the meantime, so I’m not really in a rush and would rather see this done correctly. |
@kennethormandy Let's move our conversation to #5796 for now, and later on to RFC mentioned in that issue. I'll close this PR because as mentioned above this isn't something we will be able to maintain. |
This pull request adds initial support for querying Gravity Forms, one the most popular WordPress plugins.
This scope of this PR to add initial support for forms, so you can create a form within WordPress (ex. a contact form) and then display it on your site.
Here’s an example GraphQL query are now able to do:
…and the response:
You also get access to
allWordpressGfForms
.I’m working on another React component that will help with things from there, but obviously that part it outside of the scope of Gatsby. Feel free to let me know if this is of interest to you though!
Gravity Forms also has an endpoint for entries, so in theory you would be able to accept form submissions, manage them within your headless WordPress CMS, and then have Gatsby display them in some other way.
Questions
Where is the right place for WordPress plugin wrappers to live?
The existing list of plugins that
gatsby-wordpress-source
has built-in for is short, and it probably makes sense it should stay that way. Gravity Forms is probably the only other plugin that I personally know of that makes sense to add to the list, but I’m open to moving this somewhere else if you’d like to cut that list off.At some point there may need to be some kind of hook to support any WordPress plugin rather than just a conditional in this Gatsby plugin, but that time probably isn’t yet. :)
Other things to note
Links
Thanks for taking the time to review!