fix(schema): Fix proxying invalid field names (#14108)

* Fix proxying invalid field names

* Change proxyFrom extension to proxy

* Update index.md

Author: stefanprobst

docs/blog/2019-05-17-improvements-to-schema-customization/index.md

@@ -103,6 +103,9 @@ Add resolver and resolver options (such as arguments) to the given field. There
 - `@fileByRelativePath` - connect to a File node. Same arguments. The
   difference from link is that this normalizes the relative path to be
   relative from the path where source node is found.
+- `proxy` - in case the underlying node data contains field names with
+  characters that are invalid in GraphQL, `proxy` allows to explicitly
+  proxy those properties to fields with valid field names. Takes a `from` arg.
 
 ```graphql:title=gatsby-node.js
 exports.sourceNodes = function sourceNodes({ actions }) {

packages/gatsby/src/redux/actions.js

@@ -1308,7 +1308,9 @@ import type GatsbyGraphQLType from "../schema/types/type-builders"
  * * `@fileByRelativePath` - connect to a File node. Same arguments. The
  *   difference from link is that this normalizes the relative path to be
  *   relative from the path where source node is found.
- *
+ * * `proxy` - in case the underlying node data contains field names with
+ *   characters that are invalid in GraphQL, `proxy` allows to explicitly
+ *   proxy those properties to fields with valid field names. Takes a `from` arg.
  *
  *
  * @example

packages/gatsby/src/schema/extensions/index.js

@@ -80,9 +80,14 @@ const fieldExtensions = {
     },
   },
 
-  proxyFrom: {
+  proxy: {
     description: `Proxy resolver from another field.`,
-    process(from, fieldConfig) {
+    args: {
+      from: {
+        type: GraphQLString,
+      },
+    },
+    process({ from }, fieldConfig) {
       const resolver = fieldConfig.resolve || defaultFieldResolver
       return {
         resolve(source, args, context, info) {
@@ -125,7 +130,7 @@ const processFieldExtensions = ({
     const extensions = typeComposer.getFieldExtensions(fieldName)
     Object.keys(extensions)
       .filter(name => !internalExtensionNames.includes(name))
-      .sort(a => a === `proxyFrom`) // Ensure `proxyFrom` is run last
+      .sort(a => a === `proxy`) // Ensure `proxy` is run last
       .forEach(name => {
         const { process } = fieldExtensions[name] || {}
         if (process) {

packages/gatsby/src/schema/infer/__tests__/infer.js

@@ -9,6 +9,7 @@ const { store } = require(`../../../redux`)
 const createPageDependency = require(`../../../redux/actions/add-page-dependency`)
 const { buildSchema } = require(`../../schema`)
 const { createSchemaComposer } = require(`../../schema-composer`)
+const { buildObjectType } = require(`../../types/type-builders`)
 const { TypeConflictReporter } = require(`../type-conflict-reporter`)
 require(`../../../db/__tests__/fixtures/ensure-loki`)()
 
@@ -79,7 +80,7 @@ const makeNodes = () => [
 describe(`GraphQL type inference`, () => {
   const typeConflictReporter = new TypeConflictReporter()
 
-  const buildTestSchema = async (nodes, buildSchemaArgs) => {
+  const buildTestSchema = async (nodes, buildSchemaArgs, typeDefs) => {
     store.dispatch({ type: `DELETE_CACHE` })
     nodes.forEach(node =>
       store.dispatch({ type: `CREATE_NODE`, payload: node })
@@ -89,7 +90,7 @@ describe(`GraphQL type inference`, () => {
     const schema = await buildSchema({
       schemaComposer,
       nodeStore,
-      types: [],
+      types: typeDefs || [],
       thirdPartySchemas: [],
       typeMapping: [],
       typeConflictReporter,
@@ -98,8 +99,14 @@ describe(`GraphQL type inference`, () => {
     return schema
   }
 
-  const getQueryResult = async (nodes, fragment, buildSchemaArgs) => {
-    const schema = await buildTestSchema(nodes, buildSchemaArgs)
+  const getQueryResult = async (
+    nodes,
+    fragment,
+    buildSchemaArgs,
+    extraquery = ``,
+    typeDefs
+  ) => {
+    const schema = await buildTestSchema(nodes, buildSchemaArgs, typeDefs)
     store.dispatch({ type: `SET_SCHEMA`, payload: schema })
     return graphql(
       schema,
@@ -111,6 +118,7 @@ describe(`GraphQL type inference`, () => {
             }
           }
         }
+        ${extraquery}
       }
       `,
       undefined,
@@ -342,6 +350,60 @@ describe(`GraphQL type inference`, () => {
     expect(result.data.allTest.edges[0].node._456).toEqual(nodes[0][`456`])
   })
 
+  it(`handles invalid graphql field names on explicitly defined fields`, async () => {
+    const nodes = [
+      { id: `test`, internal: { type: `Test` } },
+      {
+        id: `foo`,
+        [`field_that_needs_to_be_sanitized?`]: `foo`,
+        [`(another)_field_that_needs_to_be_sanitized`]: `bar`,
+        [`!third_field_that_needs_to_be_sanitized`]: `baz`,
+        internal: {
+          type: `Repro`,
+          contentDigest: `foo`,
+        },
+      },
+    ]
+    const typeDefs = [
+      {
+        typeOrTypeDef: buildObjectType({
+          name: `Repro`,
+          interfaces: [`Node`],
+          fields: {
+            field_that_needs_to_be_sanitized_: `String`,
+            _another__field_that_needs_to_be_sanitized: {
+              type: `String`,
+              resolve: source =>
+                source[`(another)_field_that_needs_to_be_sanitized`],
+            },
+          },
+        }),
+      },
+    ]
+
+    const result = await getQueryResult(
+      nodes,
+      `id`,
+      undefined,
+      `
+        repro {
+          field_that_needs_to_be_sanitized_
+          _another__field_that_needs_to_be_sanitized
+          _third_field_that_needs_to_be_sanitized
+        }
+      `,
+      typeDefs
+    )
+    expect(result.errors).not.toBeDefined()
+    expect(result.data.repro[`field_that_needs_to_be_sanitized_`]).toBe(`foo`)
+    expect(
+      result.data.repro[`_another__field_that_needs_to_be_sanitized`]
+    ).toBe(`bar`)
+    expect(result.data.repro[`_third_field_that_needs_to_be_sanitized`]).toBe(
+      `baz`
+    )
+  })
+
   it(`Handles priority for conflicting fields`, async () => {
     const nodes = [
       {

packages/gatsby/src/schema/infer/__tests__/merge-types.js

@@ -529,6 +529,7 @@ describe(`merges explicit and inferred type definitions`, () => {
     const typeDefs = `
       type Test implements Node @infer {
         explicitDate: Date @dateformat
+        proxied: Date @proxy(from: "explicitDate")
       }
 
       type LinkTest implements Node @infer {
@@ -546,9 +547,12 @@ describe(`merges explicit and inferred type definitions`, () => {
     expect(link.resolve).toBeDefined()
     expect(links.resolve).toBeDefined()
 
-    const { explicitDate, inferDate } = schema.getType(`Test`).getFields()
+    const { explicitDate, inferDate, proxied } = schema
+      .getType(`Test`)
+      .getFields()
     expect(explicitDate.resolve).toBeDefined()
     expect(inferDate.resolve).toBeDefined()
+    expect(proxied.resolve).toBeDefined()
   })
 
   it(`adds explicit resolvers through extensions`, async () => {

packages/gatsby/src/schema/infer/add-inferred-fields.js

@@ -116,7 +116,9 @@ const addInferredFieldsImpl = ({
               .filter(name =>
                 // It is okay to list allowed extensions explicitly here,
                 // since this is deprecated anyway and won't change.
-                [`dateformat`, `fileByRelativePath`, `link`].includes(name)
+                [`dateformat`, `fileByRelativePath`, `link`, `proxy`].includes(
+                  name
+                )
               )
               .forEach(name => {
                 if (!typeComposer.hasFieldExtension(key, name)) {
@@ -192,7 +194,7 @@ const getFieldConfig = ({
       ...fieldConfig,
       extensions: {
         ...(fieldConfig.extensions || {}),
-        proxyFrom: unsanitizedKey,
+        proxy: { from: unsanitizedKey },
       },
     }
   }