silverstripe / silverstripe-graphql

Serves Silverstripe data as GraphQL representations
BSD 3-Clause "New" or "Revised" License
52 stars 61 forks source link

PageWithDescendants tries to add non GraphQL custom page types #300

Open nspyke opened 4 years ago

nspyke commented 4 years ago

I'm using GraphQL on the frontend of my application and among other types, I've exposed a custom page type as a GraphQL type.

In the process of trying to upgrade from 4.2.5 to 4.3.6, I encountered this error Type 'AppDonationPage' is not a registered GraphQL type. The error is correct, that page type is not a GraphQL type, and I don't want it to be a GraphQL type either.

This is the relevant piece of the stack trace;

    {
      "message": "Type 'AppDonationPage' is not a registered GraphQL type",
      "code": 0,
      "file": "/app/vendor/silverstripe/graphql/src/Manager.php",
      "line": 392,
      "trace": [
        {
          "file": "/app/vendor/silverstripe/graphql/src/Scaffolding/Scaffolders/UnionScaffolder.php",
          "line": 85,
          "function": "getType",
          "class": "SilverStripe\\GraphQL\\Manager",
          "type": "->",
          "args": [
            "AppDonationPage"
          ]
        },
        {
          "function": "SilverStripe\\GraphQL\\Scaffolding\\Scaffolders\\{closure}",
          "class": "SilverStripe\\GraphQL\\Scaffolding\\Scaffolders\\UnionScaffolder",
          "type": "->",
          "args": [
            "AppDonationPage"
          ]
        },
        {
          "file": "/app/vendor/silverstripe/graphql/src/Scaffolding/Scaffolders/UnionScaffolder.php",
          "line": 86,
          "function": "array_map",
          "args": [
            {},
            [
              "Page",
              "AppDonationPage",
              "AppEventHolder",
              "AppEventPage",
              "AppGorgeBookingPage",
              "AppHomePage",
              "AppMemberRegistrationPage",
              "AppReportPage",
              "AppScoreRankPage",
              "AppWairoaBookingPage",
              "AppWidgetPage",
              "SilverStripeErrorPage",
              "SilverStripeBlog",
              "SilverStripeBlogPost",
              "SilverStripeRedirectorPage",
              "SilverStripeVirtualPage",
              "SilverStripeUserDefinedForm"
            ]
          ]
        },
        {
          "function": "SilverStripe\\GraphQL\\Scaffolding\\Scaffolders\\{closure}",
          "class": "SilverStripe\\GraphQL\\Scaffolding\\Scaffolders\\UnionScaffolder",
          "type": "->",
          "args": []
        },
        {
          "file": "/app/vendor/webonyx/graphql-php/src/Type/Definition/UnionType.php",
          "line": 61,
          "function": "call_user_func",
          "args": [
            {}
          ]
        },
        {
          "file": "/app/vendor/webonyx/graphql-php/src/Utils/TypeInfo.php",
          "line": 118,
          "function": "getTypes",
          "class": "GraphQL\\Type\\Definition\\UnionType",
          "type": "->",
          "args": []
        },
        {
          "file": "/app/vendor/webonyx/graphql-php/src/Type/Schema.php",
          "line": 228,
          "function": "extractTypes",
          "class": "GraphQL\\Utils\\TypeInfo",
          "type": "::",
          "args": [
            "PageWithDescendants",
            {
              ...
              "readAppGorgeBookingPagesConnection": "readAppGorgeBookingPagesConnection",
              "readAppGorgeBookingPagesEdge": "readAppGorgeBookingPagesEdge",
              "AppGorgeBookingPage": "AppGorgeBookingPage",
              "ReadAppGorgeBookingPageVersionSortInputType": "ReadAppGorgeBookingPageVersionSortInputType",
              "ReadAppGorgeBookingPageVersionSortFieldType": "ReadAppGorgeBookingPageVersionSortFieldType",
              "readAppGorgeBookingPageVersionConnection": "readAppGorgeBookingPageVersionConnection",
              "readAppGorgeBookingPageVersionEdge": "readAppGorgeBookingPageVersionEdge",
              "AppGorgeBookingPageVersion": "AppGorgeBookingPageVersion",
              "readPagesConnection": "readPagesConnection",
              "readPagesEdge": "readPagesEdge",
              "Page": "Page",
              "ReadPageVersionSortInputType": "ReadPageVersionSortInputType",
              "ReadPageVersionSortFieldType": "ReadPageVersionSortFieldType",
              "readPageVersionConnection": "readPageVersionConnection",
              "readPageVersionEdge": "readPageVersionEdge",
              "PageVersion": "PageVersion",
               ...
              "PageWithDescendants": "PageWithDescendants"
            }
          ]
        },
        {
          "file": "/app/vendor/webonyx/graphql-php/src/Type/Schema.php",
          "line": 196,
          "function": "collectAllTypes",
          "class": "GraphQL\\Type\\Schema",
          "type": "->",
          "args": []
        }

From the stack trace, you can see that the array_map is trying to union and scaffold all my custom page types, but the only page type that should be there is AppGorgeBookingPage.

Firstly, is there something in the documentation or something undocumented that I've missed that I can filter out / explicitly define which descendants should be scaffolded? I have had a through look and could not see anything obvious.

If there isn't then I feel that there needs to be. I should not be expected to have to define ALL my page types as GraphQL types, when I only want one page type.

Cheddam commented 4 years ago

@unclecheese Any thoughts on this? It does seem like an unreasonable limitation to me, but I'm not sure what the implications of trying to correct this are.

unclecheese commented 4 years ago

The way this is supposed to work is that any page (or dataobject with subclasses) that is exposed to GraphQL must implicitly expose its entire hierarchy, both up and down.

4.2 -> 4.3 would have jumped a major release of GraphQL. The biggest change in that major release was multi-schema support, which would have required you to update your schema config. Is there any chance that this is what's breaking your code?

nspyke commented 4 years ago

The way this is supposed to work is that any page (or dataobject with subclasses) that is exposed to GraphQL must implicitly expose its entire hierarchy, both up and down.

For my application, this is likely the problem. I don't need or want to expose the entire hierarchy. Whether it's opt-in or opt out, I should have control over what resources are exposed to the front end of the application.

4.2 -> 4.3 would have jumped a major release of GraphQL. The biggest change in that major release was multi-schema support, which would have required you to update your schema config. Is there any chance that this is what's breaking your code?

I haven't migrated it, so if it's not a BC change, then it's possible. I did not see anything about this change in the 4.3 Changelogs. I've found this, but I'm still not sure of why changing to a new schema would make any difference from the 4.2 way of doing it.