AEB-labs/graphql-weaver

Some implementations lost after weaveSchemas

igorlesnenko opened this issue · 3 comments

Before weave schema:

 _implementations: 
   { _Document: [ Navigation, Page, Page_template, Test ],
     _Linkable: 
      [ Navigation,
        Page,
        Page_template,
        Test,
        _ExternalLink,
        _FileLink,
        _ImageLink ],

Running:

 weaveSchema = await weaveSchemas({
      endpoints: [
        {
          namespace: 'cms',
          typePrefix: 'CMS',
          schema: anotherSchema,
        },
        {
          schema: localSchema,
        },
      ],
    });

Result:

_implementations: 
   { CMS_Document: [ CMSNavigation, CMSPage, CMSPage_template, CMSTest ],
     CMS_Linkable: [ CMSNavigation, CMSPage, CMSPage_template, CMSTest ],

Below fields are missing:

 _ExternalLink,
        _FileLink,
        _ImageLink

Expected:

_implementations: 
   { CMS_Document: [ CMSNavigation, CMSPage, CMSPage_template, CMSTest ],
     CMS_Linkable: [ CMSNavigation, CMSPage, CMSPage_template, CMSTest, 
CMS_ExternalLink, CMS_FileLink, CMS_ImageLink ],
Yogu commented

We remove types that are neither rechable through a field, nor are implementations of reachable interfaces. This is the implementation. Thus, it would be expected to lose types that not reachable at all. Would this apply for _FileLink and _ImageLink? I could also imagine that it's broken for implementations of interfaces that are in turn only used in implementations of interfaces, or something similar.

Could you post the SDL / type definitions for the relevant types? I'm not quite sure how to read your syntax with _implementations: etc.

@Yogu I think the issue is here:

subscription: maybeDo(schema.getSubscriptionType(), type => this.wrap(type, 'Subscription'))

We need to also provide types types: Object.values(schema.getTypeMap())

With this everything works fine

Yogu commented

Oh, so the NamespaceModule does not use transformSchema at all. I guess your fix should be fine - we only wrap the root types (and reuse the old types as the field types), so we don't need to delete unused types here.

Could you submit a pull request with this change? A testcase also would be awesome, maybe you could just copy the interfaces regression test (along with interfaces.graphql and interfaces.result.json) and add a namespace to the endpoint config. If not, a PR with just the fix would also be cool :)