Graphql 3.x-dev compatibility with upcoming CWP 2.8

[emteknetnz]

Installing kitchen-sink 2.x-dev with graphql 3.x-dev

File manager:

Installing kitchen-sink 2.7.1 and manually using graphql 3.x-dev instead of 3.4.1

Some debugging notes:

- Manager:408
- UnionScaffolder:80 - $this->type has CWPBasePage
- $_name = SilverStripeSiteTreeWithDescendants (for both exc's)
- UnionScaffolder:36
- InheritanceScaffolder:118
- :127 $tree[2] = 'CWP\CWP\PageTypes\BasePage' ($class)
- StaticSchema:109
- :112 $customTypeName = $this->mappedTypeName($class); - null
- :118 $typeName = 'CWPBasePage'
- $parts = explode('\\', $class)
    array (
        0 => 'CWP',
        1 => 'CWP',
        2 => 'PageTypes',
        3 => 'BasePage',
    )
    $typeName = sizeof($parts) > 1 // (true)
      ? $parts[0] . end($parts) // <<< CWPBasePage
      : $parts[0];

Possibly this commit? 25f654c#diff-b60efc42f855dbb4c758a6216ded97c98c345b5606e1820014afc5e174ebf473

[emteknetnz]

I'm not sure if this is the greatest solution, though it does stop asset-admin from complaining about graphql types

myproject.yml

SilverStripe\GraphQL\Manager:
  schemas:
    admin:
      typeNames:
        CWP\CWP\PageTypes\BasePage: Page

Trying to use CWP\CWP\PageTypes\BasePage: BasePage results in the error "Type 'BasePage' is not a registered GraphQL type"

My understanding is that Page will usually inherit BasePage on CWP projects, so this seems near enough

This config is really just to get graphql to not complain. I don't think there are any real world use case where graphql is used to read BasePage's specifically

We'd add this config to the cwp/cwp module where BasePage.php resides

[maxime-rainville]

Maybe this is an OK fix, but I would rather try to understand what change in-between CWP 2.7 an CWP 2.8 that this error propped up.

[emteknetnz]

I don't think anything changed in CWP. It looks like the change happened in graphql. I've provided a summary of my understanding of the graphql3 changes here. I've linked to a commit in the description of this issue where I think things changed, though I don't exactly what in there caused the CWP incompatibility.

[unclecheese]

OK, have found the issue. It's the forward compat commit as @emteknetnz identified. The issue is that, to maintain compliance with graphql 4, you can now do nodes { fieldName } as a functional equivalent to edges { node { fieldName } }. The latter is a verbose standard used for cursor-based pagination, which we're never going to have. Many GraphQL APIs support both options as we do in GraphQL 4. The way things are built in Graphql 3 means that this results in a type getting created twice -- one for each implementation of the pagination.

Fix is pretty straightforward. We just need to persist the result of getConnectionType before returning the connection. Or we could make getEdgeType take an argument for Type $connectionType. But that's a public method, so we'd need to make it optional to not break the API, in which case we'd have to fall back on creating it, which seems like way too much complexity.

I reckon in toType, we just persist the connection type to local state at that point, and then update getConnectionType to prefer that state. Half a day's effort at most.

[unclecheese]

#378

[unclecheese]

silverstripe/silverstripe-cms#2651

[emteknetnz]

Great thanks for being responsive getting this fixed Aaron, I'll look to test and merge these shortly

[emteknetnz]

@unclecheese - Worked fine when running kitchen sink 2.x-dev, however I got this when running kitchen sink 2.7.1 with a composer file as below. Scenario where this becomes relevant is non-recipe cwp projects with a graphql:^3 requirement (e.g. asset-admin 1.7.1 requites graphql:^3)

As is, this kind of implies that we still need the conflict solution to prevent older versions from upgrading to graphql 3.5

        "cwp/cwp-installer": "2.7.1",
        "cwp/agency-extensions": "^2", (all other kitchen-sink requirements are ^carets)
...
        "silverstripe/graphql": "dev-pulls/3/beta-blocker as 3.4.1",
        "silverstripe/cms": "dev-pulls/4/beta-blocker as 4.7.1",

When navigating to the file manager:

[emteknetnz]

@unclecheese Just chatted with Max, we've decided we don't need to fix this for 2.7.1, instead we'll still use the conflict solution to manage older versions. Your fix solves this for 2.x-dev / 2.8.0 so we're good to go there.

So seems like there's nothing further you need to do here. Cheers for your help!

[emteknetnz]

Just to confirm that we do not need the yml solution first proposed in a comment above. Aaron's fix is a better solution so we'll go with that instead.