-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(gatsby-source-wordpress): MediaItem.excludeFieldNames / auto exclude interface types that have no fields #37062
Changes from 8 commits
960bf21
d5e3414
16d8fd4
0db3bc5
0fc3e11
0a36964
2c6f33b
cb5cb8d
a062786
6e02e1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1493,7 +1493,6 @@ Array [ | |
Object { | ||
"fields": Array [ | ||
"avatar", | ||
"databaseId", | ||
"id", | ||
"name", | ||
"url", | ||
|
@@ -1544,7 +1543,6 @@ Array [ | |
"modifiedGmt", | ||
"slug", | ||
"status", | ||
"template", | ||
"uri", | ||
"nodeType", | ||
"parent", | ||
|
@@ -5783,7 +5781,6 @@ Array [ | |
"sourceUrl", | ||
"srcSet", | ||
"status", | ||
"template", | ||
"title", | ||
"uri", | ||
"nodeType", | ||
|
@@ -6301,12 +6298,6 @@ Array [ | |
], | ||
"name": "WpNodeWithRevisionsToContentNodeConnectionEdge", | ||
}, | ||
Object { | ||
"fields": Array [ | ||
"template", | ||
], | ||
"name": "WpNodeWithTemplate", | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These snapshot changes are because I added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
Object { | ||
"fields": Array [ | ||
"title", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { | |
fieldOfTypeWasFetched, | ||
getTypeSettingsByType, | ||
filterTypeDefinition, | ||
getTypesThatImplementInterfaceType, | ||
} from "./helpers" | ||
|
||
const unionType = typeBuilderApi => { | ||
|
@@ -47,38 +48,26 @@ const unionType = typeBuilderApi => { | |
} | ||
|
||
const interfaceType = typeBuilderApi => { | ||
const { type, schema, gatsbyNodeTypes, fieldAliases, fieldBlacklist } = | ||
typeBuilderApi | ||
const { type, schema } = typeBuilderApi | ||
const debug = type.name === `WpNode` || type.name === `Node` | ||
|
||
const state = store.getState() | ||
const { ingestibles, typeMap } = state.remoteSchema | ||
const { ingestibles } = state.remoteSchema | ||
const { nodeInterfaceTypes } = ingestibles | ||
|
||
const allTypes = typeMap.values() | ||
|
||
const implementingTypes = Array.from(allTypes) | ||
.filter( | ||
({ interfaces }) => | ||
interfaces && | ||
// find types that implement this interface type | ||
interfaces.find(singleInterface => singleInterface.name === type.name) | ||
) | ||
.map(type => typeMap.get(type.name)) | ||
.filter( | ||
type => | ||
type.kind !== `UNION` || | ||
// if this is a union type, make sure the union type has one or more member types, otherwise schema customization will throw an error | ||
(!!type.possibleTypes && !!type.possibleTypes.length) | ||
) | ||
const implementingTypes = getTypesThatImplementInterfaceType(type) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I abstracted all this code into |
||
const transformedFields = transformFields({ | ||
parentInterfacesImplementingTypes: implementingTypes, | ||
parentType: type, | ||
fields: type.fields, | ||
gatsbyNodeTypes, | ||
fieldAliases, | ||
fieldBlacklist, | ||
Comment on lines
-77
to
-79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
debug, | ||
}) | ||
|
||
if (!transformedFields) { | ||
return null | ||
} | ||
|
||
let typeDef = { | ||
name: buildTypeName(type.name), | ||
fields: transformedFields, | ||
|
@@ -144,7 +133,11 @@ const objectType = typeBuilderApi => { | |
.filter(interfaceType => { | ||
const interfaceTypeSettings = getTypeSettingsByType(interfaceType) | ||
|
||
return !interfaceTypeSettings.exclude && fieldOfTypeWasFetched(type) | ||
return ( | ||
!interfaceTypeSettings.exclude && | ||
fieldOfTypeWasFetched(type) && | ||
fieldOfTypeWasFetched(interfaceType) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interface types that were not fetched (added during query building) shouldn't appear in the schema, so this filters them out |
||
) | ||
}) | ||
.map(({ name }) => buildTypeName(name)) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,38 @@ export const fieldOfTypeWasFetched = type => { | |
return typeWasFetched | ||
} | ||
|
||
const implementingTypeCache = new Map() | ||
|
||
export const getTypesThatImplementInterfaceType = type => { | ||
if (implementingTypeCache.has(type.name)) { | ||
return implementingTypeCache.get(type.name) | ||
} | ||
|
||
const state = store.getState() | ||
const { typeMap } = state.remoteSchema | ||
|
||
const allTypes = typeMap.values() | ||
|
||
const implementingTypes = Array.from(allTypes) | ||
.filter( | ||
({ interfaces }) => | ||
interfaces && | ||
// find types that implement this interface type | ||
interfaces.find(singleInterface => singleInterface.name === type.name) | ||
) | ||
.map(type => typeMap.get(type.name)) | ||
.filter( | ||
type => | ||
type.kind !== `UNION` || | ||
// if this is a union type, make sure the union type has one or more member types, otherwise schema customization will throw an error | ||
(!!type.possibleTypes && !!type.possibleTypes.length) | ||
) | ||
|
||
implementingTypeCache.set(type.name, implementingTypes) | ||
|
||
return implementingTypes | ||
} | ||
|
||
const supportedScalars = [ | ||
`Int`, | ||
`Float`, | ||
|
@@ -84,7 +116,7 @@ export const typeIsASupportedScalar = type => { | |
return supportedScalars.includes(findTypeName(type)) | ||
} | ||
|
||
const typeSettingCache = {} | ||
const typeSettingCache = new Map() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to use a Map for caching since this function is called tons of times during schema customization and query building |
||
|
||
// retrieves plugin settings for the provided type | ||
export const getTypeSettingsByType = type => { | ||
|
@@ -94,7 +126,11 @@ export const getTypeSettingsByType = type => { | |
|
||
const typeName = findTypeName(type) | ||
|
||
const cachedTypeSettings = typeSettingCache[typeName] | ||
if (!typeName) { | ||
return {} | ||
} | ||
|
||
const cachedTypeSettings = typeSettingCache.get(typeName) | ||
|
||
if (cachedTypeSettings) { | ||
return cachedTypeSettings | ||
|
@@ -116,12 +152,12 @@ export const getTypeSettingsByType = type => { | |
if (typeSettings) { | ||
const mergedSettings = merge(__allTypeSetting, typeSettings) | ||
|
||
typeSettingCache[typeName] = mergedSettings | ||
typeSettingCache.set(typeName, mergedSettings) | ||
|
||
return mergedSettings | ||
} | ||
|
||
typeSettingCache[typeName] = __allTypeSetting | ||
typeSettingCache.set(typeName, __allTypeSetting) | ||
|
||
return __allTypeSetting | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,8 +62,7 @@ const customizeSchema = async ({ actions, schema, store: gatsbyStore }) => { | |
break | ||
case `SCALAR`: | ||
/** | ||
* custom scalar types aren't imlemented currently. | ||
* @todo make this hookable so sub-plugins or plugin options can add custom scalar support. | ||
Comment on lines
-65
to
-66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not going to do this so I removed the todo |
||
* custom scalar types aren't supported. | ||
*/ | ||
break | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,11 @@ | ||
import store from "~/store" | ||
import { typeIsExcluded } from "~/steps/ingest-remote-schema/is-excluded" | ||
import { typeIsABuiltInScalar } from "../create-schema-customization/helpers" | ||
import { findTypeName } from "~/steps/create-schema-customization/helpers" | ||
import { | ||
findTypeName, | ||
getTypesThatImplementInterfaceType, | ||
} from "~/steps/create-schema-customization/helpers" | ||
import { transformFields } from "~/steps/create-schema-customization/transform-fields" | ||
import { getPersistentCache } from "~/utils/cache" | ||
|
||
const identifyAndStoreIngestableFieldsAndTypes = async () => { | ||
|
@@ -46,6 +50,23 @@ const identifyAndStoreIngestableFieldsAndTypes = async () => { | |
continue | ||
} | ||
|
||
if (!interfaceType.fields) { | ||
continue | ||
} | ||
|
||
const typesThatImplementInterface = | ||
getTypesThatImplementInterfaceType(interfaceType) | ||
|
||
const shouldSkipInterfaceType = !transformFields({ | ||
fields: interfaceType.fields, | ||
parentType: interfaceType, | ||
parentInterfacesImplementingTypes: typesThatImplementInterface, | ||
}) | ||
|
||
if (shouldSkipInterfaceType && interfaceType.name !== `Node`) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We always need the |
||
continue | ||
} | ||
|
||
store.dispatch.remoteSchema.addFetchedType(interfaceType) | ||
|
||
if (interfaceType.fields) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
import store from "~/store" | ||
import { findTypeName } from "~/steps/create-schema-customization/helpers" | ||
import { | ||
findTypeName, | ||
getTypeSettingsByType, | ||
} from "~/steps/create-schema-customization/helpers" | ||
|
||
const typeIsExcluded = ({ pluginOptions, typeName }) => | ||
pluginOptions && | ||
|
@@ -15,9 +18,7 @@ const fieldIsExcludedOnAll = ({ pluginOptions, field }) => { | |
return !!allFieldSettings?.excludeFieldNames?.includes(field?.name) | ||
} | ||
|
||
const fieldIsExcludedOnParentType = ({ pluginOptions, field, parentType }) => { | ||
const allTypeSettings = pluginOptions.type | ||
|
||
const fieldIsExcludedOnParentType = ({ field, parentType }) => { | ||
const state = store.getState() | ||
const { typeMap } = state.remoteSchema | ||
|
||
|
@@ -27,15 +28,16 @@ const fieldIsExcludedOnParentType = ({ pluginOptions, field, parentType }) => { | |
field => field.name === `nodes` | ||
) | ||
|
||
const parentTypeNodesFieldTypeName = findTypeName(parentTypeNodesField?.type) | ||
const parentTypeNameSettings = getTypeSettingsByType(parentType) | ||
const parentTypeNodesFieldTypeNameSettings = getTypeSettingsByType( | ||
parentTypeNodesField?.type | ||
) | ||
Comment on lines
+31
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the proper way to look up plugin options type settings, not sure why I did it the other way before ;p |
||
|
||
const fieldIsExcludedOnParentType = | ||
// if this field is excluded on either the parent type | ||
allTypeSettings[parentType?.name]?.excludeFieldNames?.includes( | ||
field?.name | ||
) || | ||
parentTypeNameSettings?.excludeFieldNames?.includes(field?.name) || | ||
// or the parent type has a "nodes" field and that type has this field excluded | ||
allTypeSettings[parentTypeNodesFieldTypeName]?.excludeFieldNames?.includes( | ||
parentTypeNodesFieldTypeNameSettings?.excludeFieldNames?.includes( | ||
field?.name | ||
) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because this field was already excluded in the test site but due to a bug that this PR happens to fix it's now properly excluded in the schema.