-
Notifications
You must be signed in to change notification settings - Fork 60
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
Updates to work with WPGraphQL v1.6+ #117
Updates to work with WPGraphQL v1.6+ #117
Conversation
Release v0.2.1
Release 0.2.2
Release v0.3.1
Fix release v0.3.1
Release v0.3.3
Release v0.3.4
Release 0.3.5
- Add `eagerlyLoadType => true` to Block Types that are only exposed to the Schema as part of the `Blocks` Interface - Add condition where Block types should only be registered if they have fields.
This is amazing! @jasonbahl, between this and v0.5.* of wp-graphql-acf, WordPress is the JamStack CMS of my dreams again. Just need a couple of |
@jonsherrard that's music to my ears! Love how successful folks can be building amazing things with this stack! 🙏🏻 |
…ng, so registering a type adds a callback to the registry instead of the full Type Definition and the Schema Type Loader generates the object on demand, so this is no longer needed to be filtered
@@ -188,9 +195,11 @@ protected static function register_attributes_types($block_type, $prefix) { | |||
|
|||
register_graphql_union_type($type, [ | |||
'typeNames' => $types, | |||
'resolveType' => function ($attributes) use ($types_by_definition) { | |||
'resolveType' => function ($attributes) use ($types_by_definition, $type_registry, $types) { |
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.
@jasonbahl Not breaking anything for me as I register all my ACF fields with PHP, but when I use the ACF custom field editor to create a new group, I get a bunch of Undefined variable $type_registry in /Users/[redacted]/projects/php-projects/[redacted]/app/plugins/wp-graphql-gutenberg/src/Schema/Types/BlockTypes.php on line 198
errors rendered into the GraphQL settings block
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.
This is using 1.6.2 of wp-graphql, and this 0.3.9 tag.
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.
Ah @jasonbahl yes, we can remove the $type_registry
from that line as it's not needed here. Was left over as I was working on some updates where I thought we might need it in the ResolveType
callback.
I'll follow up with another quick change
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.
Sorry for the report-onslaught - Another little breadcrumb:
My gatsby-source-wordpress@4
site doesn't like the graphQL schema any more either:
success gatsby-source-wordpress fetch root fields - 0.329s
success gatsby-source-wordpress pull updates since last build - 1.328s
success Checking for changed pages - 0.001s
success source and transform nodes - 1.864s
ERROR
Missing onError handler for invocation 'building-schema', error was 'Error: Type with name "WpBlockAttributesObject" does not exists'. Stacktrace was 'Error: Type with name "WpBlockAttributesObject" does not exists
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.
@jonsherrard, this might be related to Gatsby Source WordPress. I believe this open issue might be related: wp-graphql/wp-graphql#2046
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.
Ah yes, bit late here and bamboozled, gatsby-source-wordpress hasn't worked properly since about 1.5.6-ish if I recall.
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.
Oh, interesting. Would be nice to pinpoint which release specifically caused this issue.
My thought was that the 1.6.0 release of WPGraphQL which introduced Lazy Type loading was causing the issue as Gatsby would have to be a bit more specific about it's Introspection queries and re-assignment of Types in the Gatsby GraphQL Schema.
But if it was a 1.5.* release that caused Gatsby to no longer build, then there might be another issue that I'm unaware of.
If you're able to do some testing with 1.5.4, 1.5.5, 1.5.6, 1.5.7, 1.5.8, 1.5.9 and 1.6.2, that would be 👌🏻
I do think there was an issue with 1.5.6 that caused one issue, but that was patched in 1.5.9
I believe the 1.6.* issues are different and related to how Gatsby does introspection and builds the Schema in Gatsby.
Since WPGraphQL is doing Lazy Type loading, I believe Gatsby will need to be more specific about the Types that it asks for from WPGraphQL and being more intentional about how it maps those types to the Gatsby Schema.
This PR updates some code to play nice with the WPGraphQL 1.6 Lazy Type loading
I've tested this PR against WPGraphQL v1.5.8 and 1.6.2
closes #116
BEFORE:
Before this PR, 1.6+ would cause GraphiQL to never load with a
resolveType()
error.AFTER:
With this PR, I can load GraphiQL, browse the Types, construct queries and execute queries.
Faster Load Times!:
The main focus of WPGraphQL v1.6 release was to improve performance all around, and with WPGraphQL for Gutenberg active, things could get a bit slow.
With WPGraphQL v1.6 and this PR in place for WPGraphQL for Gutenberg, we're seeing significant performance wins 👏🏻
Before
With WPGraphQL + Gutenberg on WPGraphQL v1.5.8 the Schema would take 9.53s to load in GraphiQL for me:
After
With WPGraphQL for Gutenberg (with this PR) and WPGraphQL v1.6.2 the Schema loads in GraphiQL in just 2.27s!