Skip to content

Commit 0bc63fe

Browse files
vladarvladar
authoredSep 14, 2020
feat(gatsby): merge GraphQL types defined by different plugins (with a warning) (#26864)
* feat(gatsby): merge GraphQL types defined by different plugins (with a warning) * Improve warning messages * test overriding built-in types
·
v0.0.0-6babel-plugin-remove-graphql-queries@2.9.19
1 parent bf61854 commit 0bc63fe

File tree

2 files changed

+135
-53
lines changed

2 files changed

+135
-53
lines changed
 

‎packages/gatsby/src/schema/__tests__/build-schema.js

+94-16
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ describe(`Build schema`, () => {
685685
)
686686
})
687687

688-
it(`does not merge plugin-defined type with type defined by other plugin`, async () => {
688+
it(`warns when merging plugin-defined type with type defined by other plugin`, async () => {
689689
createTypes(
690690
`type PluginDefined implements Node { foo: Int, baz: PluginDefinedNested }
691691
type PluginDefinedNested { foo: Int }`,
@@ -703,28 +703,32 @@ describe(`Build schema`, () => {
703703
const schema = await buildSchema()
704704
const nestedFields = schema.getType(`PluginDefinedNested`).getFields()
705705
const fields = schema.getType(`PluginDefined`).getFields()
706-
expect(Object.keys(nestedFields)).toEqual([`foo`])
706+
expect(Object.keys(nestedFields)).toEqual([`foo`, `bar`])
707707
expect(Object.keys(fields)).toEqual([
708708
`foo`,
709709
`baz`,
710+
`bar`,
711+
`qux`,
710712
`id`,
711713
`parent`,
712714
`children`,
713715
`internal`,
714716
])
715717
expect(report.warn).toHaveBeenCalledWith(
716-
`Plugin \`some-other-gatsby-plugin\` tried to define the GraphQL type ` +
718+
`Plugin \`some-other-gatsby-plugin\` has customized the GraphQL type ` +
717719
`\`PluginDefinedNested\`, which has already been defined by the plugin ` +
718-
`\`some-gatsby-plugin\`.`
720+
`\`some-gatsby-plugin\`. ` +
721+
`This could potentially cause conflicts.`
719722
)
720723
expect(report.warn).toHaveBeenCalledWith(
721-
`Plugin \`some-other-gatsby-plugin\` tried to define the GraphQL type ` +
724+
`Plugin \`some-other-gatsby-plugin\` has customized the GraphQL type ` +
722725
`\`PluginDefined\`, which has already been defined by the plugin ` +
723-
`\`some-gatsby-plugin\`.`
726+
`\`some-gatsby-plugin\`. ` +
727+
`This could potentially cause conflicts.`
724728
)
725729
})
726730

727-
it(`does not merge plugin-defined type (Type Builder) with type defined by other plugin`, async () => {
731+
it(`warns when merging plugin-defined type (Type Builder) with type defined by other plugin`, async () => {
728732
createTypes(
729733
`type PluginDefined implements Node { foo: Int, baz: PluginDefinedNested }
730734
type PluginDefinedNested { foo: Int }`,
@@ -759,28 +763,34 @@ describe(`Build schema`, () => {
759763
const schema = await buildSchema()
760764
const nestedFields = schema.getType(`PluginDefinedNested`).getFields()
761765
const fields = schema.getType(`PluginDefined`).getFields()
762-
expect(Object.keys(nestedFields)).toEqual([`foo`])
766+
expect(Object.keys(nestedFields)).toEqual([`foo`, `bar`])
763767
expect(Object.keys(fields)).toEqual([
764768
`foo`,
765769
`baz`,
770+
`bar`,
771+
`qux`,
766772
`id`,
767773
`parent`,
768774
`children`,
769775
`internal`,
770776
])
771777
expect(report.warn).toHaveBeenCalledWith(
772-
`Plugin \`some-other-gatsby-plugin\` tried to define the GraphQL type ` +
778+
`Plugin \`some-other-gatsby-plugin\` has customized the GraphQL type ` +
773779
`\`PluginDefinedNested\`, which has already been defined by the plugin ` +
774-
`\`some-gatsby-plugin\`.`
780+
`\`some-gatsby-plugin\`. ` +
781+
`This could potentially cause conflicts.`
775782
)
776783
expect(report.warn).toHaveBeenCalledWith(
777-
`Plugin \`some-other-gatsby-plugin\` tried to define the GraphQL type ` +
784+
`Plugin \`some-other-gatsby-plugin\` has customized the GraphQL type ` +
778785
`\`PluginDefined\`, which has already been defined by the plugin ` +
779-
`\`some-gatsby-plugin\`.`
786+
`\`some-gatsby-plugin\`. ` +
787+
`This could potentially cause conflicts.`
780788
)
781789
})
782790

783-
it(`does not merge plugin-defined type (graphql-js) with type defined by other plugin`, async () => {
791+
// FIXME: Fails with: Error: Cannot get field 'foo' from type 'PluginDefinedNested'. Field does not exist.
792+
// same as "merges user-defined type (graphql-js) with plugin-defined type"
793+
it.skip(`warns when merging plugin-defined type (graphql-js) with type defined by other plugin`, async () => {
784794
createTypes(
785795
`type PluginDefined implements Node { foo: Int, baz: PluginDefinedNested }
786796
type PluginDefinedNested { foo: Int }`,
@@ -807,27 +817,95 @@ describe(`Build schema`, () => {
807817
const schema = await buildSchema()
808818
const nestedFields = schema.getType(`PluginDefinedNested`).getFields()
809819
const fields = schema.getType(`PluginDefined`).getFields()
810-
expect(Object.keys(nestedFields)).toEqual([`foo`])
820+
expect(Object.keys(nestedFields)).toEqual([`foo`, `bar`])
811821
expect(Object.keys(fields)).toEqual([
812822
`foo`,
813823
`baz`,
824+
`bar`,
825+
`qux`,
814826
`id`,
815827
`parent`,
816828
`children`,
817829
`internal`,
818830
])
819831
expect(report.warn).toHaveBeenCalledWith(
820-
`Plugin \`some-other-gatsby-plugin\` tried to define the GraphQL type ` +
832+
`Plugin \`some-other-gatsby-plugin\` has customized the GraphQL type ` +
821833
`\`PluginDefinedNested\`, which has already been defined by the plugin ` +
822834
`\`some-gatsby-plugin\`.`
823835
)
824836
expect(report.warn).toHaveBeenCalledWith(
825-
`Plugin \`some-other-gatsby-plugin\` tried to define the GraphQL type ` +
837+
`Plugin \`some-other-gatsby-plugin\` has customized the GraphQL type ` +
826838
`\`PluginDefined\`, which has already been defined by the plugin ` +
827839
`\`some-gatsby-plugin\`.`
828840
)
829841
})
830842

843+
it(`warns when merging plugin-defined type with built-in type`, async () => {
844+
createTypes(`type SitePage implements Node { bar: Int }`, {
845+
name: `some-gatsby-plugin`,
846+
})
847+
const schema = await buildSchema()
848+
const fields = schema.getType(`SitePage`).getFields()
849+
expect(Object.keys(fields)).toEqual([
850+
`path`,
851+
`component`,
852+
`internalComponentName`,
853+
`componentChunkName`,
854+
`matchPath`,
855+
`bar`,
856+
`id`,
857+
`parent`,
858+
`children`,
859+
`internal`,
860+
])
861+
expect(report.warn).toHaveBeenCalledWith(
862+
`Plugin \`some-gatsby-plugin\` has customized the built-in Gatsby GraphQL type ` +
863+
`\`SitePage\`. This is allowed, but could potentially cause conflicts.`
864+
)
865+
})
866+
867+
it(`warns when merging plugin-defined type (Type Builder) with built-in type`, async () => {
868+
createTypes(
869+
[
870+
buildObjectType({
871+
name: `SitePage`,
872+
interfaces: [`Node`],
873+
extensions: {
874+
infer: false,
875+
},
876+
fields: {
877+
bar: `Int`,
878+
},
879+
}),
880+
],
881+
{
882+
name: `some-gatsby-plugin`,
883+
}
884+
)
885+
const schema = await buildSchema()
886+
const fields = schema.getType(`SitePage`).getFields()
887+
expect(Object.keys(fields)).toEqual([
888+
`path`,
889+
`component`,
890+
`internalComponentName`,
891+
`componentChunkName`,
892+
`matchPath`,
893+
`bar`,
894+
`id`,
895+
`parent`,
896+
`children`,
897+
`internal`,
898+
])
899+
expect(report.warn).toHaveBeenCalledWith(
900+
`Plugin \`some-gatsby-plugin\` has customized the built-in Gatsby GraphQL type ` +
901+
`\`SitePage\`. This is allowed, but could potentially cause conflicts.`
902+
)
903+
})
904+
905+
it.todo(
906+
`warns when merging plugin-defined type (graphql-js) with built-in type`
907+
)
908+
831909
it(`extends fieldconfigs when merging types`, async () => {
832910
createTypes(
833911
buildObjectType({

‎packages/gatsby/src/schema/schema.js

+41-37
Original file line numberDiff line numberDiff line change
@@ -338,52 +338,56 @@ const mergeTypes = ({
338338
createdFrom,
339339
parentSpan,
340340
}) => {
341-
// Only allow user or plugin owning the type to extend already existing type.
341+
// The merge is considered safe when a user or a plugin owning the type extend this type
342+
// TODO: add proper conflicts detection and reporting (on the field level)
342343
const typeOwner = typeComposer.getExtension(`plugin`)
343-
if (
344+
const isSafeMerge =
344345
!plugin ||
345346
plugin.name === `default-site-plugin` ||
346347
plugin.name === typeOwner
347-
) {
348-
if (type instanceof ObjectTypeComposer) {
349-
mergeFields({ typeComposer, fields: type.getFields() })
350-
type.getInterfaces().forEach(iface => typeComposer.addInterface(iface))
351-
} else if (type instanceof InterfaceTypeComposer) {
352-
mergeFields({ typeComposer, fields: type.getFields() })
353-
} else if (type instanceof GraphQLObjectType) {
354-
mergeFields({
355-
typeComposer,
356-
fields: defineFieldMapToConfig(type.getFields()),
357-
})
358-
type.getInterfaces().forEach(iface => typeComposer.addInterface(iface))
359-
} else if (type instanceof GraphQLInterfaceType) {
360-
mergeFields({
361-
typeComposer,
362-
fields: defineFieldMapToConfig(type.getFields()),
363-
})
364-
}
365348

366-
if (isNamedTypeComposer(type)) {
367-
typeComposer.extendExtensions(type.getExtensions())
349+
if (!isSafeMerge) {
350+
if (typeOwner) {
351+
report.warn(
352+
`Plugin \`${plugin.name}\` has customized the GraphQL type ` +
353+
`\`${typeComposer.getTypeName()}\`, which has already been defined ` +
354+
`by the plugin \`${typeOwner}\`. ` +
355+
`This could potentially cause conflicts.`
356+
)
357+
} else {
358+
report.warn(
359+
`Plugin \`${plugin.name}\` has customized the built-in Gatsby GraphQL type ` +
360+
`\`${typeComposer.getTypeName()}\`. ` +
361+
`This is allowed, but could potentially cause conflicts.`
362+
)
368363
}
364+
}
369365

370-
addExtensions({ schemaComposer, typeComposer, plugin, createdFrom })
366+
if (type instanceof ObjectTypeComposer) {
367+
mergeFields({ typeComposer, fields: type.getFields() })
368+
type.getInterfaces().forEach(iface => typeComposer.addInterface(iface))
369+
} else if (type instanceof InterfaceTypeComposer) {
370+
mergeFields({ typeComposer, fields: type.getFields() })
371+
} else if (type instanceof GraphQLObjectType) {
372+
mergeFields({
373+
typeComposer,
374+
fields: defineFieldMapToConfig(type.getFields()),
375+
})
376+
type.getInterfaces().forEach(iface => typeComposer.addInterface(iface))
377+
} else if (type instanceof GraphQLInterfaceType) {
378+
mergeFields({
379+
typeComposer,
380+
fields: defineFieldMapToConfig(type.getFields()),
381+
})
382+
}
371383

372-
return true
373-
} else if (typeOwner) {
374-
report.warn(
375-
`Plugin \`${plugin.name}\` tried to define the GraphQL type ` +
376-
`\`${typeComposer.getTypeName()}\`, which has already been defined ` +
377-
`by the plugin \`${typeOwner}\`.`
378-
)
379-
return false
380-
} else {
381-
report.warn(
382-
`Plugin \`${plugin.name}\` tried to define built-in Gatsby GraphQL type ` +
383-
`\`${typeComposer.getTypeName()}\``
384-
)
385-
return false
384+
if (isNamedTypeComposer(type)) {
385+
typeComposer.extendExtensions(type.getExtensions())
386386
}
387+
388+
addExtensions({ schemaComposer, typeComposer, plugin, createdFrom })
389+
390+
return true
387391
}
388392

389393
const processAddedType = ({

0 commit comments

Comments
 (0)
Please sign in to comment.