From efdb0b76eb270968d5baa28edc7db634a81a672f Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 30 Sep 2024 15:36:55 +0100 Subject: [PATCH 01/15] Remove manyToMany warnings --- .../src/plugins/PgIndexBehaviorsPlugin.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/graphile-build/graphile-build-pg/src/plugins/PgIndexBehaviorsPlugin.ts b/graphile-build/graphile-build-pg/src/plugins/PgIndexBehaviorsPlugin.ts index db3486729..8dce612ec 100644 --- a/graphile-build/graphile-build-pg/src/plugins/PgIndexBehaviorsPlugin.ts +++ b/graphile-build/graphile-build-pg/src/plugins/PgIndexBehaviorsPlugin.ts @@ -12,6 +12,11 @@ declare global { isIndexed?: boolean; } } + namespace GraphileBuild { + interface BehaviorStrings { + manyToMany: true; + } + } } export const PgIndexBehaviorsPlugin: GraphileConfig.Plugin = { @@ -87,6 +92,15 @@ export const PgIndexBehaviorsPlugin: GraphileConfig.Plugin = { }, }, schema: { + behaviorRegistry: { + add: { + // HACK: this impacts a community plugin and isn't part of core. + manyToMany: { + entities: [], + description: "(Legacy support hack - many-to-many relationship)", + }, + }, + }, entityBehavior: { pgCodecAttribute: { inferred: { @@ -115,7 +129,7 @@ export const PgIndexBehaviorsPlugin: GraphileConfig.Plugin = { "-single", // HACK: this impacts a community plugin and isn't part of core. - "-manyToMany" as GraphileBuild.BehaviorString, + "-manyToMany", ); } return newBehavior; From 0df17112300a8ea391dfd220c5f05d362ceaa58a Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 30 Sep 2024 15:37:11 +0100 Subject: [PATCH 02/15] docs(changeset): Fix issue with manyToMany behavior warnings --- .changeset/great-cameras-double.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/great-cameras-double.md diff --git a/.changeset/great-cameras-double.md b/.changeset/great-cameras-double.md new file mode 100644 index 000000000..8a562518f --- /dev/null +++ b/.changeset/great-cameras-double.md @@ -0,0 +1,5 @@ +--- +"graphile-build-pg": patch +--- + +Fix issue with manyToMany behavior warnings From 9208c251522e56d0b7cb0ca8d66f0abe1582229f Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 30 Sep 2024 15:00:17 +0100 Subject: [PATCH 03/15] V4 preset should use Amber preset --- postgraphile/postgraphile/src/presets/v4.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/postgraphile/postgraphile/src/presets/v4.ts b/postgraphile/postgraphile/src/presets/v4.ts index ef68ecc87..d88004d35 100644 --- a/postgraphile/postgraphile/src/presets/v4.ts +++ b/postgraphile/postgraphile/src/presets/v4.ts @@ -8,6 +8,7 @@ import { PgV4BehaviorPlugin } from "../plugins/PgV4BehaviorPlugin.js"; import { PgV4InflectionPlugin } from "../plugins/PgV4InflectionPlugin.js"; import { PgV4SimpleSubscriptionsPlugin } from "../plugins/PgV4SimpleSubscriptionsPlugin.js"; import { PgV4SmartTagsPlugin } from "../plugins/PgV4SmartTagsPlugin.js"; +import PostGraphileAmberPreset from "./amber.js"; export { PgV4BehaviorPlugin, @@ -214,6 +215,7 @@ export const makeV4Preset = ( const graphiqlPath = options.graphiqlRoute ?? "/graphiql"; const eventStreamPath = options.eventStreamRoute ?? `${graphqlPath}/stream`; return { + extends: [PostGraphileAmberPreset], plugins: [ PgV4InflectionPlugin, PgV4SmartTagsPlugin, From 282eb909f3e2dc74fb262714248d29056464fb1d Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 30 Sep 2024 15:00:52 +0100 Subject: [PATCH 04/15] docs(changeset): PostGraphile V4 preset should automatically include the Amber preset on which it relies. --- .changeset/nine-ladybugs-guess.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/nine-ladybugs-guess.md diff --git a/.changeset/nine-ladybugs-guess.md b/.changeset/nine-ladybugs-guess.md new file mode 100644 index 000000000..fee637803 --- /dev/null +++ b/.changeset/nine-ladybugs-guess.md @@ -0,0 +1,6 @@ +--- +"postgraphile": patch +--- + +PostGraphile V4 preset should automatically include the Amber preset on which it +relies. From baa5cbcf021bc826551cedf5691f8a163486448b Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 30 Sep 2024 16:58:16 +0100 Subject: [PATCH 05/15] Change the mechanics of how disablePlugins works --- utils/graphile-config/src/resolvePresets.ts | 55 ++++++++++++++++----- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 28561526f..2974b06d6 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -197,9 +197,17 @@ function resolvePreset( ); } } - const { extends: presets = [] } = preset; - const basePreset = resolvePresets(presets, false); - mergePreset(basePreset, preset); + } catch (e) { + throw new Error( + `Error occurred when resolving preset: ${e}\nPreset: ${inspect(preset)}`, + ); + } + + const { extends: presets = [], ...rest } = preset; + const basePreset = resolvePresets(presets, false); + + try { + mergePreset(basePreset, rest); const disabled = basePreset.disablePlugins; if (disabled) { @@ -233,24 +241,45 @@ function resolvePreset( */ function mergePreset( targetPreset: GraphileConfig.ResolvedPreset, - sourcePreset: GraphileConfig.Preset, + sourcePreset: GraphileConfig.ResolvedPreset, ): void { if (targetPreset.extends != null && targetPreset.extends.length !== 0) { throw new Error("First argument to mergePreset must be a resolved preset"); } + + const sourcePluginNames = sourcePreset.plugins?.map((p) => p.name) ?? []; + const addedAndDisabled = sourcePreset.disablePlugins + ? sourcePluginNames.filter((addedPluginName) => + sourcePreset.disablePlugins!.includes(addedPluginName), + ) + : []; + if (addedAndDisabled.length > 0) { + throw new Error( + `A preset may not both add a plugin and disable that same plugin ('${addedAndDisabled.join( + "', '", + )}')`, + ); + } + + const disablePlugins = [ + ...new Set([ + // Remove the previously disabled plugins where we've explicitly re-added the plugin + ...(targetPreset.disablePlugins?.filter( + (pluginName) => !sourcePluginNames.includes(pluginName), + ) ?? []), + // Explicitly add our new disablePlugins + ...(sourcePreset.disablePlugins ?? []), + ]), + ]; + const plugins = new Set([ ...(targetPreset.plugins || []), ...(sourcePreset.plugins || []), ]); - targetPreset.plugins = [...plugins]; - if (sourcePreset.disablePlugins) { - targetPreset.disablePlugins = [ - ...new Set([ - ...(targetPreset.disablePlugins ?? []), - ...(sourcePreset.disablePlugins ?? []), - ]), - ]; - } + // Copy the unique plugins that are not disabled + targetPreset.plugins = [...plugins].filter( + (p) => !disablePlugins.includes(p.name), + ); const targetScopes = Object.keys(targetPreset).filter(isScopeKeyForPreset); const sourceScopes = Object.keys(sourcePreset).filter(isScopeKeyForPreset); const scopes = [...new Set([...targetScopes, ...sourceScopes])]; From aed70abb644da5f76692aacbc6e9fe13b5abcde7 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 11:19:05 +0100 Subject: [PATCH 06/15] Track 'seenPlugins' and warn if attempt to disablePlugin for a plugin not seen --- utils/graphile-config/src/resolvePresets.ts | 92 +++++++++++++-------- 1 file changed, 57 insertions(+), 35 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 2974b06d6..5a760dbf8 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -40,16 +40,48 @@ try { export function isResolvedPreset( preset: GraphileConfig.Preset, ): preset is GraphileConfig.ResolvedPreset { - return (preset.plugins && preset.extends?.length === 0) || false; + return ( + (preset.plugins && + preset.extends?.length === 0 && + (!preset.disablePlugins || + !preset.plugins || + !preset.plugins.some((p) => + preset.disablePlugins!.includes(p.name), + ))) || + false + ); } /** * Given a list of presets, resolves the presets and returns the resulting * ResolvedPreset (which does not have any `extends`). */ -export function resolvePresets( +export function resolvePresets(presets: ReadonlyArray) { + // TODO: seenPlugins is wrong; the plugins should only ever grow, and disablePlugins itself should apply at the very end. + const seenPlugins = new Set(); + const resolvedPreset = resolvePresetsInternal(presets, seenPlugins, 0); + + const seenNames = [...seenPlugins].map((p) => p.name); + const disabledButNotSeen = resolvedPreset.disablePlugins?.filter( + (n) => !seenNames.includes(n), + ); + if (disabledButNotSeen?.length) { + console.warn( + `One or more of the plugin(s) entered in your preset's 'disablePlugins' list was never seen - perhaps you have misspelled them?\n${disabledButNotSeen + .map((p) => ` - ${p}`) + .join("\n")}\nThe list of know plugins is:\n ${ + seenNames.join(", ") ?? "-" + }`, + ); + } + + return resolvedPreset; +} + +function resolvePresetsInternal( presets: ReadonlyArray, - withAssertions = true, + seenPlugins: Set, + depth: number, ): GraphileConfig.ResolvedPreset { if (presets.length === 1) { // Maybe it's already resolved? @@ -59,9 +91,15 @@ export function resolvePresets( } } const finalPreset = blankResolvedPreset(); + let presetIndex = 0; for (const preset of presets) { - const resolvedPreset = resolvePreset(preset); - mergePreset(finalPreset, resolvedPreset); + const resolvedPreset = resolvePresetInternal( + preset, + seenPlugins, + depth + 1, + ); + mergePreset(finalPreset, resolvedPreset, seenPlugins, depth); + presetIndex++; } if (finalPreset.plugins) { @@ -71,18 +109,6 @@ export function resolvePresets( ); } - if (withAssertions) { - if (finalPreset.disablePlugins && finalPreset.disablePlugins.length > 0) { - console.warn( - `One or more of the plugin(s) entered in your preset's 'disablePlugins' list was not found:\n${finalPreset.disablePlugins - .map((p) => ` - ${p}`) - .join("\n")}\nThe list of know plugins is:\n ${ - finalPreset.plugins?.map((p) => p.name).join(", ") ?? "-" - }`, - ); - } - } - return finalPreset; } @@ -163,8 +189,10 @@ function isForbiddenPluginKey(key: string): boolean { * * @internal */ -function resolvePreset( +function resolvePresetInternal( preset: GraphileConfig.Preset, + seenPlugins: Set, + depth: number, ): GraphileConfig.ResolvedPreset { if (!isGraphileConfigPreset(preset)) { throw new Error( @@ -204,25 +232,10 @@ function resolvePreset( } const { extends: presets = [], ...rest } = preset; - const basePreset = resolvePresets(presets, false); + const basePreset = resolvePresetsInternal(presets, seenPlugins, depth + 1); try { - mergePreset(basePreset, rest); - - const disabled = basePreset.disablePlugins; - if (disabled) { - const plugins = new Set(basePreset.plugins); - const remaining = new Set(disabled); - for (const plugin of plugins) { - assertPlugin(plugin); - if (remaining.has(plugin.name)) { - remaining.delete(plugin.name); - plugins.delete(plugin); - } - } - basePreset.plugins = [...plugins]; - basePreset.disablePlugins = [...remaining]; - } + mergePreset(basePreset, rest, seenPlugins, depth); return basePreset; } catch (e) { throw new Error( @@ -242,6 +255,8 @@ function resolvePreset( function mergePreset( targetPreset: GraphileConfig.ResolvedPreset, sourcePreset: GraphileConfig.ResolvedPreset, + seenPlugins: Set, + depth: number, ): void { if (targetPreset.extends != null && targetPreset.extends.length !== 0) { throw new Error("First argument to mergePreset must be a resolved preset"); @@ -271,11 +286,18 @@ function mergePreset( ...(sourcePreset.disablePlugins ?? []), ]), ]; + targetPreset.disablePlugins = disablePlugins; const plugins = new Set([ ...(targetPreset.plugins || []), ...(sourcePreset.plugins || []), ]); + if (sourcePreset.plugins) { + for (const plugin of sourcePreset.plugins) { + seenPlugins.add(plugin); + } + } + // Copy the unique plugins that are not disabled targetPreset.plugins = [...plugins].filter( (p) => !disablePlugins.includes(p.name), From 18a0f8f4a2da37ac133859825c5684fb27e9d054 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 11:22:00 +0100 Subject: [PATCH 07/15] Just keep all the plugins about --- utils/graphile-config/src/resolvePresets.ts | 28 ++++++--------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 5a760dbf8..8f13fb1e1 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -57,11 +57,9 @@ export function isResolvedPreset( * ResolvedPreset (which does not have any `extends`). */ export function resolvePresets(presets: ReadonlyArray) { - // TODO: seenPlugins is wrong; the plugins should only ever grow, and disablePlugins itself should apply at the very end. - const seenPlugins = new Set(); - const resolvedPreset = resolvePresetsInternal(presets, seenPlugins, 0); + const resolvedPreset = resolvePresetsInternal(presets, 0); - const seenNames = [...seenPlugins].map((p) => p.name); + const seenNames = resolvedPreset.plugins?.map((p) => p.name) ?? []; const disabledButNotSeen = resolvedPreset.disablePlugins?.filter( (n) => !seenNames.includes(n), ); @@ -80,7 +78,6 @@ export function resolvePresets(presets: ReadonlyArray) { function resolvePresetsInternal( presets: ReadonlyArray, - seenPlugins: Set, depth: number, ): GraphileConfig.ResolvedPreset { if (presets.length === 1) { @@ -93,12 +90,8 @@ function resolvePresetsInternal( const finalPreset = blankResolvedPreset(); let presetIndex = 0; for (const preset of presets) { - const resolvedPreset = resolvePresetInternal( - preset, - seenPlugins, - depth + 1, - ); - mergePreset(finalPreset, resolvedPreset, seenPlugins, depth); + const resolvedPreset = resolvePresetInternal(preset, depth + 1); + mergePreset(finalPreset, resolvedPreset, depth); presetIndex++; } @@ -191,7 +184,6 @@ function isForbiddenPluginKey(key: string): boolean { */ function resolvePresetInternal( preset: GraphileConfig.Preset, - seenPlugins: Set, depth: number, ): GraphileConfig.ResolvedPreset { if (!isGraphileConfigPreset(preset)) { @@ -232,10 +224,10 @@ function resolvePresetInternal( } const { extends: presets = [], ...rest } = preset; - const basePreset = resolvePresetsInternal(presets, seenPlugins, depth + 1); + const basePreset = resolvePresetsInternal(presets, depth + 1); try { - mergePreset(basePreset, rest, seenPlugins, depth); + mergePreset(basePreset, rest, depth); return basePreset; } catch (e) { throw new Error( @@ -255,8 +247,7 @@ function resolvePresetInternal( function mergePreset( targetPreset: GraphileConfig.ResolvedPreset, sourcePreset: GraphileConfig.ResolvedPreset, - seenPlugins: Set, - depth: number, + _depth: number, ): void { if (targetPreset.extends != null && targetPreset.extends.length !== 0) { throw new Error("First argument to mergePreset must be a resolved preset"); @@ -292,11 +283,6 @@ function mergePreset( ...(targetPreset.plugins || []), ...(sourcePreset.plugins || []), ]); - if (sourcePreset.plugins) { - for (const plugin of sourcePreset.plugins) { - seenPlugins.add(plugin); - } - } // Copy the unique plugins that are not disabled targetPreset.plugins = [...plugins].filter( From 8c9fc4fcbb04dbbec37c899970358024f5dec228 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 11:28:12 +0100 Subject: [PATCH 08/15] Revert "Just keep all the plugins about" This reverts commit 18a0f8f4a2da37ac133859825c5684fb27e9d054. --- utils/graphile-config/src/resolvePresets.ts | 28 +++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 8f13fb1e1..5a760dbf8 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -57,9 +57,11 @@ export function isResolvedPreset( * ResolvedPreset (which does not have any `extends`). */ export function resolvePresets(presets: ReadonlyArray) { - const resolvedPreset = resolvePresetsInternal(presets, 0); + // TODO: seenPlugins is wrong; the plugins should only ever grow, and disablePlugins itself should apply at the very end. + const seenPlugins = new Set(); + const resolvedPreset = resolvePresetsInternal(presets, seenPlugins, 0); - const seenNames = resolvedPreset.plugins?.map((p) => p.name) ?? []; + const seenNames = [...seenPlugins].map((p) => p.name); const disabledButNotSeen = resolvedPreset.disablePlugins?.filter( (n) => !seenNames.includes(n), ); @@ -78,6 +80,7 @@ export function resolvePresets(presets: ReadonlyArray) { function resolvePresetsInternal( presets: ReadonlyArray, + seenPlugins: Set, depth: number, ): GraphileConfig.ResolvedPreset { if (presets.length === 1) { @@ -90,8 +93,12 @@ function resolvePresetsInternal( const finalPreset = blankResolvedPreset(); let presetIndex = 0; for (const preset of presets) { - const resolvedPreset = resolvePresetInternal(preset, depth + 1); - mergePreset(finalPreset, resolvedPreset, depth); + const resolvedPreset = resolvePresetInternal( + preset, + seenPlugins, + depth + 1, + ); + mergePreset(finalPreset, resolvedPreset, seenPlugins, depth); presetIndex++; } @@ -184,6 +191,7 @@ function isForbiddenPluginKey(key: string): boolean { */ function resolvePresetInternal( preset: GraphileConfig.Preset, + seenPlugins: Set, depth: number, ): GraphileConfig.ResolvedPreset { if (!isGraphileConfigPreset(preset)) { @@ -224,10 +232,10 @@ function resolvePresetInternal( } const { extends: presets = [], ...rest } = preset; - const basePreset = resolvePresetsInternal(presets, depth + 1); + const basePreset = resolvePresetsInternal(presets, seenPlugins, depth + 1); try { - mergePreset(basePreset, rest, depth); + mergePreset(basePreset, rest, seenPlugins, depth); return basePreset; } catch (e) { throw new Error( @@ -247,7 +255,8 @@ function resolvePresetInternal( function mergePreset( targetPreset: GraphileConfig.ResolvedPreset, sourcePreset: GraphileConfig.ResolvedPreset, - _depth: number, + seenPlugins: Set, + depth: number, ): void { if (targetPreset.extends != null && targetPreset.extends.length !== 0) { throw new Error("First argument to mergePreset must be a resolved preset"); @@ -283,6 +292,11 @@ function mergePreset( ...(targetPreset.plugins || []), ...(sourcePreset.plugins || []), ]); + if (sourcePreset.plugins) { + for (const plugin of sourcePreset.plugins) { + seenPlugins.add(plugin); + } + } // Copy the unique plugins that are not disabled targetPreset.plugins = [...plugins].filter( From 01f61fbb288e984c1529907055a63ef2101d9a42 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 11:52:44 +0100 Subject: [PATCH 09/15] Hoist more logic to top level --- utils/graphile-config/src/resolvePresets.ts | 47 ++++++++++----------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 5a760dbf8..14078e083 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -57,9 +57,22 @@ export function isResolvedPreset( * ResolvedPreset (which does not have any `extends`). */ export function resolvePresets(presets: ReadonlyArray) { - // TODO: seenPlugins is wrong; the plugins should only ever grow, and disablePlugins itself should apply at the very end. + if (presets.length === 1) { + // Maybe it's already resolved? + const preset = presets[0]; + if (preset && isResolvedPreset(preset)) { + return preset; + } + } + const seenPlugins = new Set(); const resolvedPreset = resolvePresetsInternal(presets, seenPlugins, 0); + if (resolvedPreset.plugins) { + resolvedPreset.plugins = sortWithBeforeAfterProvides( + resolvedPreset.plugins, + "name", + ); + } const seenNames = [...seenPlugins].map((p) => p.name); const disabledButNotSeen = resolvedPreset.disablePlugins?.filter( @@ -83,15 +96,7 @@ function resolvePresetsInternal( seenPlugins: Set, depth: number, ): GraphileConfig.ResolvedPreset { - if (presets.length === 1) { - // Maybe it's already resolved? - const preset = presets[0]; - if (preset && isResolvedPreset(preset)) { - return preset; - } - } const finalPreset = blankResolvedPreset(); - let presetIndex = 0; for (const preset of presets) { const resolvedPreset = resolvePresetInternal( preset, @@ -99,16 +104,7 @@ function resolvePresetsInternal( depth + 1, ); mergePreset(finalPreset, resolvedPreset, seenPlugins, depth); - presetIndex++; - } - - if (finalPreset.plugins) { - finalPreset.plugins = sortWithBeforeAfterProvides( - finalPreset.plugins, - "name", - ); } - return finalPreset; } @@ -256,13 +252,19 @@ function mergePreset( targetPreset: GraphileConfig.ResolvedPreset, sourcePreset: GraphileConfig.ResolvedPreset, seenPlugins: Set, - depth: number, + _depth: number, ): void { + const sourcePluginNames: string[] = []; + if (sourcePreset.plugins) { + for (const plugin of sourcePreset.plugins) { + seenPlugins.add(plugin); + sourcePluginNames.push(plugin.name); + } + } if (targetPreset.extends != null && targetPreset.extends.length !== 0) { throw new Error("First argument to mergePreset must be a resolved preset"); } - const sourcePluginNames = sourcePreset.plugins?.map((p) => p.name) ?? []; const addedAndDisabled = sourcePreset.disablePlugins ? sourcePluginNames.filter((addedPluginName) => sourcePreset.disablePlugins!.includes(addedPluginName), @@ -292,11 +294,6 @@ function mergePreset( ...(targetPreset.plugins || []), ...(sourcePreset.plugins || []), ]); - if (sourcePreset.plugins) { - for (const plugin of sourcePreset.plugins) { - seenPlugins.add(plugin); - } - } // Copy the unique plugins that are not disabled targetPreset.plugins = [...plugins].filter( From 937a883b62180cf6d32659a5026507b1dc997f38 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 12:08:17 +0100 Subject: [PATCH 10/15] Revert change that results in different plugin ordering --- utils/graphile-config/src/resolvePresets.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 14078e083..671f6c13f 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -67,12 +67,6 @@ export function resolvePresets(presets: ReadonlyArray) { const seenPlugins = new Set(); const resolvedPreset = resolvePresetsInternal(presets, seenPlugins, 0); - if (resolvedPreset.plugins) { - resolvedPreset.plugins = sortWithBeforeAfterProvides( - resolvedPreset.plugins, - "name", - ); - } const seenNames = [...seenPlugins].map((p) => p.name); const disabledButNotSeen = resolvedPreset.disablePlugins?.filter( @@ -105,6 +99,12 @@ function resolvePresetsInternal( ); mergePreset(finalPreset, resolvedPreset, seenPlugins, depth); } + if (finalPreset.plugins) { + finalPreset.plugins = sortWithBeforeAfterProvides( + finalPreset.plugins, + "name", + ); + } return finalPreset; } From 43e4f7af5e0e6bd4d192cb05f904759f660772bf Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 12:12:00 +0100 Subject: [PATCH 11/15] Restore plugin assertion --- utils/graphile-config/src/resolvePresets.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 671f6c13f..63682917e 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -257,6 +257,7 @@ function mergePreset( const sourcePluginNames: string[] = []; if (sourcePreset.plugins) { for (const plugin of sourcePreset.plugins) { + assertPlugin(plugin); seenPlugins.add(plugin); sourcePluginNames.push(plugin.name); } From cc0941731a1679bc04ce7b7fd4254009bb5f1f62 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 12:14:50 +0100 Subject: [PATCH 12/15] docs(changeset): Overhaul the way in which `graphile-config` presets work such that including a preset at two different layers shouldn't result in unexpected behavior. --- .changeset/polite-cherries-exercise.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/polite-cherries-exercise.md diff --git a/.changeset/polite-cherries-exercise.md b/.changeset/polite-cherries-exercise.md new file mode 100644 index 000000000..9e9db73d5 --- /dev/null +++ b/.changeset/polite-cherries-exercise.md @@ -0,0 +1,8 @@ +--- +"graphile-build-pg": patch +"postgraphile": patch +"graphile-config": patch +--- + +Overhaul the way in which `graphile-config` presets work such that including a +preset at two different layers shouldn't result in unexpected behavior. From 7bf21b91f994a7915f1469cce70e2b8b47d953f0 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 12:18:06 +0100 Subject: [PATCH 13/15] Trust disabled plugins from already resolved presets --- utils/graphile-config/src/resolvePresets.ts | 34 +++++++++++++-------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index 63682917e..b422f1382 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -65,19 +65,18 @@ export function resolvePresets(presets: ReadonlyArray) { } } - const seenPlugins = new Set(); - const resolvedPreset = resolvePresetsInternal(presets, seenPlugins, 0); + const seenPluginNames = new Set(); + const resolvedPreset = resolvePresetsInternal(presets, seenPluginNames, 0); - const seenNames = [...seenPlugins].map((p) => p.name); const disabledButNotSeen = resolvedPreset.disablePlugins?.filter( - (n) => !seenNames.includes(n), + (n) => !seenPluginNames.has(n), ); if (disabledButNotSeen?.length) { console.warn( `One or more of the plugin(s) entered in your preset's 'disablePlugins' list was never seen - perhaps you have misspelled them?\n${disabledButNotSeen .map((p) => ` - ${p}`) .join("\n")}\nThe list of know plugins is:\n ${ - seenNames.join(", ") ?? "-" + [...seenPluginNames].join(", ") ?? "-" }`, ); } @@ -87,17 +86,22 @@ export function resolvePresets(presets: ReadonlyArray) { function resolvePresetsInternal( presets: ReadonlyArray, - seenPlugins: Set, + seenPluginNames: Set, depth: number, ): GraphileConfig.ResolvedPreset { const finalPreset = blankResolvedPreset(); for (const preset of presets) { + if (isResolvedPreset(preset) && preset.disablePlugins) { + for (const p of preset.disablePlugins) { + seenPluginNames.add(p); + } + } const resolvedPreset = resolvePresetInternal( preset, - seenPlugins, + seenPluginNames, depth + 1, ); - mergePreset(finalPreset, resolvedPreset, seenPlugins, depth); + mergePreset(finalPreset, resolvedPreset, seenPluginNames, depth); } if (finalPreset.plugins) { finalPreset.plugins = sortWithBeforeAfterProvides( @@ -187,7 +191,7 @@ function isForbiddenPluginKey(key: string): boolean { */ function resolvePresetInternal( preset: GraphileConfig.Preset, - seenPlugins: Set, + seenPluginNames: Set, depth: number, ): GraphileConfig.ResolvedPreset { if (!isGraphileConfigPreset(preset)) { @@ -228,10 +232,14 @@ function resolvePresetInternal( } const { extends: presets = [], ...rest } = preset; - const basePreset = resolvePresetsInternal(presets, seenPlugins, depth + 1); + const basePreset = resolvePresetsInternal( + presets, + seenPluginNames, + depth + 1, + ); try { - mergePreset(basePreset, rest, seenPlugins, depth); + mergePreset(basePreset, rest, seenPluginNames, depth); return basePreset; } catch (e) { throw new Error( @@ -251,14 +259,14 @@ function resolvePresetInternal( function mergePreset( targetPreset: GraphileConfig.ResolvedPreset, sourcePreset: GraphileConfig.ResolvedPreset, - seenPlugins: Set, + seenPluginNames: Set, _depth: number, ): void { const sourcePluginNames: string[] = []; if (sourcePreset.plugins) { for (const plugin of sourcePreset.plugins) { assertPlugin(plugin); - seenPlugins.add(plugin); + seenPluginNames.add(plugin.name); sourcePluginNames.push(plugin.name); } } From f404892ca8b0abb95e4e51c538d00fa59ac012a8 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 12:19:10 +0100 Subject: [PATCH 14/15] Fix release notes --- .changeset/nine-ladybugs-guess.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/nine-ladybugs-guess.md b/.changeset/nine-ladybugs-guess.md index fee637803..b250a6540 100644 --- a/.changeset/nine-ladybugs-guess.md +++ b/.changeset/nine-ladybugs-guess.md @@ -2,5 +2,5 @@ "postgraphile": patch --- -PostGraphile V4 preset should automatically include the Amber preset on which it +PostGraphile V4 preset now automatically includes the Amber preset on which it relies. From 052c00b82b0524e0dc86d76e99d2638fe692501c Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 1 Oct 2024 12:54:52 +0100 Subject: [PATCH 15/15] Improve error message when nested --- utils/graphile-config/src/resolvePresets.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/utils/graphile-config/src/resolvePresets.ts b/utils/graphile-config/src/resolvePresets.ts index b422f1382..e2b25dd23 100644 --- a/utils/graphile-config/src/resolvePresets.ts +++ b/utils/graphile-config/src/resolvePresets.ts @@ -103,12 +103,14 @@ function resolvePresetsInternal( ); mergePreset(finalPreset, resolvedPreset, seenPluginNames, depth); } + if (finalPreset.plugins) { finalPreset.plugins = sortWithBeforeAfterProvides( finalPreset.plugins, "name", ); } + return finalPreset; } @@ -227,7 +229,10 @@ function resolvePresetInternal( } } catch (e) { throw new Error( - `Error occurred when resolving preset: ${e}\nPreset: ${inspect(preset)}`, + `Error occurred when resolving preset:\n ${String(e).replace( + /\n/g, + "\n ", + )}\nPreset: ${inspect(preset)}`, ); } @@ -243,7 +248,10 @@ function resolvePresetInternal( return basePreset; } catch (e) { throw new Error( - `Error occurred when resolving preset: ${e}\nPreset: ${inspect(preset)}`, + `Error occurred when resolving preset:\n ${String(e).replace( + /\n/g, + "\n ", + )}\nPreset: ${inspect(preset)}`, ); } }