Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

TransformRootFields + RenameObjectFields not working together #27

Closed
m-sanders opened this issue Nov 26, 2019 · 9 comments
Closed

TransformRootFields + RenameObjectFields not working together #27

m-sanders opened this issue Nov 26, 2019 · 9 comments

Comments

@m-sanders
Copy link

m-sanders commented Nov 26, 2019

Thank you for this library. We’ve found that schema delegation is not ready for the primetime and we’re continuing with stitching! I have however, encountered a problem with TransformRootFields + RenameObjectFields transforms in the latest version (7.1.4) and wondered if you could help.

I am using Prismic as a headless CMS and below is a snippet of a query to illustrate the bug:

{
  ingredient(uid: "sugar", lang: "en-us") {
    title
    camel_case
  }
  allIngredients(first: 1, lang: "en-us") {
    edges {
      node {
        title
        camel_case
      }
    }
  }
}

To match the rest of our application we’d like to apply some transformations:

  • Change allIngredients -> ingredients
  • Rename camel_case -> camelCase
  • Apply a default value to lang based on the context.

I am creating the schema with the below:

const link = new PrismicLink({
  uri: PRISMIC_URL
});
const schema = makeRemoteExecutableSchema({
  schema: await introspectSchema(link),
  link
});

return transformSchema(schema, [
  new TransformRootFields((_operation, fieldName, field) => {
    // `allSomethingContentTypes` -> `somethingContentTypes`.
    let name = fieldName;
    if (fieldName.match(/^all/)) {
      name = name[3].toLowerCase() + fieldName.substr(4);
    }

    // Set any root fields with a `lang` arg to default to the correct locale.
    const config = fieldToFieldConfig(field);
    if (config.args.lang !== undefined) {
      config.args.lang.defaultValue = locale.langtag.toLowerCase();
    }
    return { name, field: config };
  }),
  new RenameObjectFields((_typeName, fieldName) => {
    // prismic using leading underscores for special fields. Leave them alone.
    if (fieldName[0] === "_") return fieldName;
    // snake_case -> camelCase
    return fieldName.replace(/([-_][a-z])/gi, $1 => {
      return $1
        .toUpperCase()
        .replace("-", "")
        .replace("_", "");
    });
  })

However the below query returns inconsistent results. ingredient.camelCase gets correctly transformed but ingredient.edges.node.camelCase does not and returns null

{
  ingredient(uid: "sugar") {
    title
    camelCase
  }
  ingredients(first: 1) {
    edges {
      node {
        title
        camelCase
      }
    }
  }
}

Result:

{
  "data": {
    "ingredient": {
      "title": "Sugar",
      "camelCase": [
        {
          "type": "paragraph",
          "text": "Camel Camel Camel",
          "spans": []
        }
      ]
    },
    "ingredients": {
      "edges": [
        {
          "node": {
            "title": "Sugar",
            "camelCase": null
          }
        }
      ]
    }
  }
}

If I remove the rename on TransformRootFields, it works as expected. Thoughts?

@yaacovCR
Copy link
Owner

Can you set up a repo with reproduction and I can investigate?

Do you know whether the correct query is being sent or not?

There is a typo in ingredient.edgets.node.camelCase above, assuming unrelated.

@m-sanders
Copy link
Author

Yes, that’s just a typo. I’ll setup a minimal example.

@m-sanders
Copy link
Author

Hello @yaacovCR

I have an example at: https://github.com/m-sanders/graphql-tools-fork-27

@yaacovCR
Copy link
Owner

Awesome, will check out, thanks.

@yaacovCR
Copy link
Owner

Thank you again for providing the clear bug report and reproduction.

This is partially addressed by v7.2.0.

Following new test adapted from your repository reproduction passes if and only if root fields are transformed last:

    const schema = transformSchema(itemSchema, [
      new RenameObjectFields((_typeName, fieldName) => {
        if (fieldName === 'camel_case') {
          return 'camelCase';
        }
        return fieldName;
      }),
      new RenameRootFields((_operation, fieldName) => {
        if (fieldName === 'allItems') {
          return 'items';
        }
        return fieldName;
      }),
    ]);

The transformations will fail when listed in the reverse order. Why?

Well, request transformations are applied in the reverse order, as the request is in the final schema format, and must be sequentially modified until it matches the original schema.

Root field names, however, are hard coded into the outer wrapping schema's delegateToSchema function calls (see code in and around https://github.com/yaacovCR/graphql-tools-fork/blob/master/src/transforms/transformSchema.ts#L36), so that renaming of root fields is not accomplished by request transformation, but rather by simply renaming the outer schema's root fields, which always map to the original schema's original root field names.

That means that all other transformations will always see the original schemas root field names no matter the order of transformations given, but will only expect to see the original schemas root field names if that transformation is listed last (as request transformations happen in reverse order).

Things weren't working before no matter the order of specification because in this fork visitSchema works by modifying the schema in place (as it was unified with the SchemaDirectiveVisitor pattern), so intermediate schema representations were not being saved, and the requests transformations were always being performed with the final schema representation, which did not understand the original root field names.

The requirement for root field name transformation to be listed last could be lifted if RenameRootFields were to regenerate the outer wrapping schemas proxying resolvers with the new root fields and provide a request transformation that transforms them back to the old root fields, in which case now that intermediate schema representations are being saved properly, everything should work correctly. That seems unnecessary, as it is easier and less compute cycles to just list root field transformations last. This should, at the very lest, be better documented.

I am leaving this issue open for now as this is only a partial fix, and you (or others!) may have some input as to whether this is sufficient.

@yaacovCR
Copy link
Owner

Thought about this more, looks like we can relax requirement pretty easily actually, we could let delegateToSchema default on no field name to info.fieldName and give RenameRootFields/TransformRootFields a proper requestTransform to change that at runtime. Will work on that time permitting, now that you have workaround hopefully.

@m-sanders
Copy link
Author

Thank you @yaacovCR for the detailed reply, the quick fix and the workaround. I have upgraded to the latest version and re-ordered the transforms. It is now working as expected.

@yaacovCR
Copy link
Owner

Thanks to you for finding bug!

@yaacovCR
Copy link
Owner

yaacovCR commented Dec 1, 2019

In terms of above plan, turned out to not be as straightforward as I thought. TransformObjectFields actually has to change info.fieldName so that defaultMergedResolver (or checkResultAndHandleErrors for root fields) can find the correct responseKey. Will require more thought...

yaacovCR added a commit that referenced this issue Jan 21, 2020
…form ordering

Fixes #27, now possible because info object is not modified, see 1676ab0
yaacovCR added a commit that referenced this issue Jan 21, 2020
…form ordering

Fixes #27, now possible because info object is not modified, see 1676ab0
yaacovCR added a commit that referenced this issue Feb 17, 2020
…form ordering

Fixes #27, now possible because info object is not modified, see 1676ab0
yaacovCR added a commit that referenced this issue Feb 27, 2020
…form ordering

Fixes #27, now possible because info object is not modified, see 1676ab0
yaacovCR added a commit that referenced this issue Mar 26, 2020
…form ordering

Fixes #27, now possible because info object is not modified, see 1676ab0
yaacovCR added a commit that referenced this issue Mar 26, 2020
…form ordering

Fixes #27, now possible because info object is not modified, see 1676ab0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants