Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log type conflics when inferring graphql types #3905

Merged
merged 15 commits into from
Apr 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/gatsby-source-filesystem/src/create-file-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ exports.createFileNode = async (pathToFile, pluginOptions = {}) => {
internal = {
contentDigest,
type: `Directory`,
description: `Directory "${path.relative(process.cwd(), slashed)}"`,
}
} else {
const contentDigest = await md5File(slashedFile.absolutePath)
internal = {
contentDigest,
mediaType: mime.lookup(slashedFile.ext),
type: `File`,
description: `File "${path.relative(process.cwd(), slashed)}"`,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ async function processRemoteNode({ url, store, cache, createNode, auth = {} }) {

// Create the file node.
const fileNode = await createFileNode(filename, {})

fileNode.internal.description = `File "${url}"`
// Override the default plugin as gatsby-source-filesystem needs to
// be the owner of File nodes or there'll be conflicts if any other
// File nodes are created through normal usages of
Expand Down
2 changes: 2 additions & 0 deletions packages/gatsby/src/bootstrap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ module.exports = async (args: BootstrapArgs) => {
await require(`../schema`)()
activity.end()

require(`../schema/type-conflict-reporter`).printConflicts()

// Extract queries
activity = report.activityTimer(`extract queries from components`)
activity.start()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ exports.onCreatePage = ({ page, boundActionCreators }) => {
.createHash(`md5`)
.update(JSON.stringify(page))
.digest(`hex`),
description:
page.pluginCreatorId === `Plugin default-site-plugin`
? `Your site's "gatsby-node.js"`
: page.pluginCreatorId,
},
})
}
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby/src/joi-schemas/joi.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const nodeSchema = Joi.object()
owner: Joi.string().required(),
fieldOwners: Joi.array(),
content: Joi.string().allow(``),
description: Joi.string(),
}),
})
.unknown()
5 changes: 5 additions & 0 deletions packages/gatsby/src/redux/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,10 @@ const typeOwners = {}
* @param {string} node.internal.contentDigest the digest for the content
* of this node. Helps Gatsby avoid doing extra work on data that hasn't
* changed.
* @param {string} node.internal.description An optional field. Human
* readable description of what this node represent / its source. It will
* be displayed when type conflicts are found, making it easier to find
* and correct type conflicts.
* @example
* createNode({
* // Data for the node.
Expand All @@ -525,6 +529,7 @@ const typeOwners = {}
* .digest(`hex`),
* mediaType: `text/markdown`, // optional
* content: JSON.stringify(fieldData), // optional
* description: `Cool Service: "Title of entry"`, // optional
* }
* })
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ Object {
"name": Object {
"field": "name",
},
"nestedArrays": Object {
"field": "nestedArrays",
},
"objectsInArray": Object {
"field": "objectsInArray",
},
}
`;

Expand All @@ -45,21 +51,33 @@ Object {
],
"context": Object {
"nestedObject": Object {
"someOtherProperty": 3,
"someOtherProperty": 1,
},
},
"date": "2006-07-22T22:39:53.000Z",
"emptyArray": null,
"frontmatter": Object {
"blue": 10010,
"blue": 100,
"circle": "happy",
"date": "2006-07-22T22:39:53.000Z",
"draft": false,
"title": "The world of slash and adventure",
"title": "The world of dash and adventure",
},
"hair": 4,
"hair": 1,
"iAmNull": null,
"key-with..unsupported-values": true,
"name": "The Mad Wax",
"name": "The Mad Max",
"nestedArrays": Array [
Array [
1,
],
],
"objectsInArray": Array [
Object {
"field1": true,
"field2": 1,
"field3": "foo",
},
],
}
`;
144 changes: 134 additions & 10 deletions packages/gatsby/src/schema/__tests__/data-tree-utils-test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
const {
extractFieldExamples,
getExampleValues,
buildFieldEnumValues,
clearTypeExampleValues,
INVALID_VALUE,
} = require(`../data-tree-utils`)
const {
typeConflictReporter,
TypeConflictEntry,
} = require(`../type-conflict-reporter`)

describe(`Gatsby data tree utils`, () => {
beforeEach(() => {
clearTypeExampleValues()
})

const nodes = [
{
name: `The Mad Max`,
Expand All @@ -13,6 +22,8 @@ describe(`Gatsby data tree utils`, () => {
"key-with..unsupported-values": true,
emptyArray: [],
anArray: [1, 2, 3, 4],
nestedArrays: [[1, 2, 3], [4, 5, 6]],
objectsInArray: [{ field1: true }, { field2: 1 }],
frontmatter: {
date: `2006-07-22T22:39:53.000Z`,
title: `The world of dash and adventure`,
Expand All @@ -29,6 +40,8 @@ describe(`Gatsby data tree utils`, () => {
emptyArray: [undefined, null],
anArray: [1, 2, 5, 4],
iAmNull: null,
nestedArrays: [[1, 2, 3]],
objectsInArray: [{ field3: `foo` }],
frontmatter: {
date: `2006-07-22T22:39:53.000Z`,
title: `The world of slash and adventure`,
Expand Down Expand Up @@ -83,32 +96,32 @@ describe(`Gatsby data tree utils`, () => {
]

it(`builds field examples from an array of nodes`, () => {
expect(extractFieldExamples(nodes)).toMatchSnapshot()
expect(getExampleValues(nodes)).toMatchSnapshot()
})

it(`null fields should have a null value`, () => {
expect(extractFieldExamples(nodes).iAmNull).toBeNull()
expect(getExampleValues(nodes).iAmNull).toBeNull()
})

it(`should not mutate the nodes`, () => {
extractFieldExamples(nodes)
getExampleValues(nodes)
expect(nodes[0].context.nestedObject).toBeNull()
expect(nodes[1].context.nestedObject.someOtherProperty).toEqual(1)
expect(nodes[2].context.nestedObject.someOtherProperty).toEqual(2)
expect(nodes[3].context.nestedObject.someOtherProperty).toEqual(3)
})

it(`turns empty or sparse arrays to null`, () => {
expect(extractFieldExamples(nodes).emptyArray).toBeNull()
expect(extractFieldExamples(nodes).hair).toBeDefined()
expect(getExampleValues(nodes).emptyArray).toBeNull()
expect(getExampleValues(nodes).hair).toBeDefined()
})

it(`build enum values for fields from array on nodes`, () => {
expect(buildFieldEnumValues(nodes)).toMatchSnapshot()
})

it(`turns polymorphic fields null`, () => {
let example = extractFieldExamples([
let example = getExampleValues([
{ foo: null },
{ foo: [1] },
{ foo: { field: 1 } },
Expand All @@ -117,26 +130,137 @@ describe(`Gatsby data tree utils`, () => {
})

it(`handles polymorphic arrays`, () => {
let example = extractFieldExamples([
let example = getExampleValues([
{ foo: [[`foo`, `bar`]] },
{ foo: [{ field: 1 }] },
])
expect(example.foo).toBe(INVALID_VALUE)
})

it(`doesn't confuse empty fields for polymorhpic ones`, () => {
let example = extractFieldExamples([
let example = getExampleValues([
{ foo: { bar: 1 } },
{ foo: null },
{ foo: { field: 1 } },
])
expect(example.foo).toEqual({ field: 1, bar: 1 })

example = extractFieldExamples([
example = getExampleValues([
{ foo: [{ bar: 1 }] },
{ foo: null },
{ foo: [{ field: 1 }, { baz: 1 }] },
])
expect(example.foo).toEqual([{ field: 1, bar: 1, baz: 1 }])
})
})

describe(`Type conflicts`, () => {
let addConflictSpy = jest.spyOn(typeConflictReporter, `addConflict`)
let addConflictExampleSpy = jest.spyOn(
TypeConflictEntry.prototype,
`addExample`
)

beforeEach(() => {
clearTypeExampleValues()
addConflictExampleSpy.mockReset()
})

afterAll(() => {
addConflictSpy.mockRestore()
addConflictExampleSpy.mockRestore()
})

it(`Doesn't report conflicts if there are none`, () => {
const nodes = [
{
id: `id1`,
string: `string`,
number: 5,
boolean: true,
arrayOfStrings: [`string1`],
},
{
id: `id2`,
string: `other string`,
number: 3.5,
boolean: false,
arrayOfStrings: null,
},
]

getExampleValues({ nodes, type: `NoConflict` })

expect(addConflictExampleSpy).not.toBeCalled()
})

it(`Report type conflicts and its origin`, () => {
const nodes = [
{
id: `id1`,
stringOrNumber: `string`,
number: 5,
boolean: true,
arrayOfStrings: [`string1`],
},
{
id: `id2`,
stringOrNumber: 5,
number: 3.5,
boolean: false,
arrayOfStrings: null,
},
]

getExampleValues({ nodes, type: `Conflict_1` })

expect(addConflictSpy).toBeCalled()
expect(addConflictSpy).toBeCalledWith(
`Conflict_1.stringOrNumber`,
expect.any(Array)
)

// expect(addConflictExampleSpy).toBeCalled()
expect(addConflictExampleSpy).toHaveBeenCalledTimes(2)
expect(addConflictExampleSpy).toBeCalledWith(
expect.objectContaining({
value: nodes[0].stringOrNumber,
type: `string`,
parent: nodes[0],
})
)
expect(addConflictExampleSpy).toBeCalledWith(
expect.objectContaining({
value: nodes[1].stringOrNumber,
type: `number`,
parent: nodes[1],
})
)
})

it(`Report conflict when array has mixed types and its origin`, () => {
const nodes = [
{
id: `id1`,
arrayOfMixedType: [`string1`, 5, `string2`, true],
},
]

getExampleValues({ nodes, type: `Conflict_2` })
expect(addConflictSpy).toBeCalled()
expect(addConflictSpy).toBeCalledWith(
`Conflict_2.arrayOfMixedType`,
expect.any(Array)
)

expect(addConflictExampleSpy).toBeCalled()
expect(addConflictExampleSpy).toHaveBeenCalledTimes(1)
expect(addConflictExampleSpy).toBeCalledWith(
expect.objectContaining({
value: nodes[0].arrayOfMixedType,
type: `array<boolean|number|string>`,
parent: nodes[0],
})
)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const {
inferInputObjectStructureFromNodes,
} = require(`../infer-graphql-input-fields`)
const createSortField = require(`../create-sort-field`)
const { clearTypeExampleValues } = require(`../data-tree-utils`)

function queryResult(nodes, query, { types = [] } = {}) {
const nodeType = new GraphQLObjectType({
Expand All @@ -29,7 +30,7 @@ function queryResult(nodes, query, { types = [] } = {}) {
nodeType,
connectionFields: () =>
buildConnectionFields({
name: `Test`,
name,
nodes,
nodeObjectType: nodeType,
}),
Expand Down Expand Up @@ -75,6 +76,10 @@ function queryResult(nodes, query, { types = [] } = {}) {
return graphql(schema, query)
}

beforeEach(() => {
clearTypeExampleValues()
})

describe(`GraphQL Input args`, () => {
const nodes = [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const {
} = require(`graphql`)
const path = require(`path`)
const normalizePath = require(`normalize-path`)

const { clearTypeExampleValues } = require(`../data-tree-utils`)
const { inferObjectStructureFromNodes } = require(`../infer-graphql-type`)

function queryResult(nodes, fragment, { types = [] } = {}) {
Expand Down Expand Up @@ -48,6 +48,10 @@ function queryResult(nodes, fragment, { types = [] } = {}) {
)
}

beforeEach(() => {
clearTypeExampleValues()
})

describe(`GraphQL type inferance`, () => {
const nodes = [
{
Expand Down
Loading