wp-graphql / wp-graphql-tax-query

Adds `tax_query` support to postObject connection queries using WP_Query
46 stars 16 forks source link

Update to work with WPGraphQL v1.6.12+ (BREAKING) #32

Closed jasonbahl closed 1 year ago

jasonbahl commented 2 years ago

This PR brings the plugin up to date to work with WPGraphQL 1.6.12 and newer.

Instead of Types being named after the connection they're on, they're root type names.

Ex:

Before:

query getPosts(
  $operator: RootQueryToPostConnectionWhereArgsTaxQueryOperator
) {
  posts(
    where: {
      taxQuery: {
        taxArray: [
          {
            terms: ["cat1", "cat2"]
            taxonomy: CATEGORY
            operator: $operator
            field: SLUG
          }
        ]
      }
    }
  ) {
    nodes {
      slug
    }
  }
}

After:

query getPosts(
  $operator: TaxQueryOperator
) {
  posts(
    where: {
      taxQuery: {
        taxArray: [
          {
            terms: ["cat1", "cat2"]
            taxonomy: CATEGORY
            operator: $operator
            field: SLUG
          }
        ]
      }
    }
  ) {
    nodes {
      slug
    }
  }
}
jasonbahl commented 2 years ago

closes #31 closes #27

justlevine commented 2 years ago

@jasonbahl personally I'm all for simplifying the type names as a breaking change but I worry this PR is working around the bug indicated by the linked issues.

I'm not by my PC, but if this PR fixes things, that implies either the types are not being registered at the right point in the lifecycle or the type name isn't always available in graphql_input_fields.

Not suggesting to hold off on merging , but we should try to replicate the bug in wp-graphql core so it can be addressed (either via a codefix or a docs note).

jasonbahl commented 2 years ago

@justlevine so this used to work before Lazy Loading types because the entire schema was generated on each request.

So that means that each request would generate all the Input Types, triggering the graphql_input_fields filter, and then generate all the Types for this plugin.

Since Lazy Loading was introduced, Types are loaded on demand.

So, a query like this one:

query getPosts(
  $operator: RootQueryToPostConnectionWhereArgsTaxQueryOperator
) {
  posts(
    where: {
      taxQuery: {
        taxArray: [
          {
            terms: ["cat1", "cat2"]
            taxonomy: CATEGORY
            operator: $operator
            field: SLUG
          }
        ]
      }
    }
  ) {
    nodes {
      slug
    }
  }
}

Will try and load the RootQueryToPostConnectionWhereArgsTaxQueryOperator Type as part of the validation of the query, but it can't find the type, because that Type is only loaded when graphql_input_fields hook is executed, and it will not have executed by time we need to ask for this Type, hence the errors.

The graphql_input_fields can/should be used to filter fields onto an input type, but should not be used as a hook to register Types on.

We got away with this pre-lazy loading because all types were generated, including those generated as a side effect of other Types, but this was probably always a bad practice (that I didn't fully realize was a bad practice myself)

justlevine commented 2 years ago

The graphql_input_fields can/should be used to filter fields onto an input type, but should not be used as a hook to register Types on.

We got away with this pre-lazy loading because all types were generated, including those generated as a side effect of other Types, but this was probably always a bad practice (that I didn't fully realize was a bad practice myself)

Good to know! Explains why I'm been trying for days to implement that pattern and getting nowhere 💀

KoduVaal commented 2 years ago

Hi @jasonbahl @justlevine< when we will be merging this? we still waiting to updating the wp-graphql plugin because of this