Skip to content

Commit

Permalink
Merge branch 'main' into assert-not-async
Browse files Browse the repository at this point in the history
  • Loading branch information
benjie authored Oct 1, 2024
2 parents b0865d1 + b97469b commit 374f846
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 49 deletions.
5 changes: 5 additions & 0 deletions .changeset/great-cameras-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"graphile-build-pg": patch
---

Fix issue with manyToMany behavior warnings
6 changes: 6 additions & 0 deletions .changeset/nine-ladybugs-guess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"postgraphile": patch
---

PostGraphile V4 preset now automatically includes the Amber preset on which it
relies.
8 changes: 8 additions & 0 deletions .changeset/polite-cherries-exercise.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ declare global {
isIndexed?: boolean;
}
}
namespace GraphileBuild {
interface BehaviorStrings {
manyToMany: true;
}
}
}

export const PgIndexBehaviorsPlugin: GraphileConfig.Plugin = {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions postgraphile/postgraphile/src/presets/v4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -214,6 +215,7 @@ export const makeV4Preset = (
const graphiqlPath = options.graphiqlRoute ?? "/graphiql";
const eventStreamPath = options.eventStreamRoute ?? `${graphqlPath}/stream`;
return {
extends: [PostGraphileAmberPreset],
plugins: [
PgV4InflectionPlugin,
PgV4SmartTagsPlugin,
Expand Down
161 changes: 113 additions & 48 deletions utils/graphile-config/src/resolvePresets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,28 +40,68 @@ 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(
presets: ReadonlyArray<GraphileConfig.Preset>,
withAssertions = true,
): GraphileConfig.ResolvedPreset {
export function resolvePresets(presets: ReadonlyArray<GraphileConfig.Preset>) {
if (presets.length === 1) {
// Maybe it's already resolved?
const preset = presets[0];
if (preset && isResolvedPreset(preset)) {
return preset;
}
}

const seenPluginNames = new Set<string>();
const resolvedPreset = resolvePresetsInternal(presets, seenPluginNames, 0);

const disabledButNotSeen = resolvedPreset.disablePlugins?.filter(
(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 ${
[...seenPluginNames].join(", ") ?? "-"
}`,
);
}

return resolvedPreset;
}

function resolvePresetsInternal(
presets: ReadonlyArray<GraphileConfig.Preset>,
seenPluginNames: Set<string>,
depth: number,
): GraphileConfig.ResolvedPreset {
const finalPreset = blankResolvedPreset();
for (const preset of presets) {
const resolvedPreset = resolvePreset(preset);
mergePreset(finalPreset, resolvedPreset);
if (isResolvedPreset(preset) && preset.disablePlugins) {
for (const p of preset.disablePlugins) {
seenPluginNames.add(p);
}
}
const resolvedPreset = resolvePresetInternal(
preset,
seenPluginNames,
depth + 1,
);
mergePreset(finalPreset, resolvedPreset, seenPluginNames, depth);
}

if (finalPreset.plugins) {
Expand All @@ -71,18 +111,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;
}

Expand Down Expand Up @@ -163,8 +191,10 @@ function isForbiddenPluginKey(key: string): boolean {
*
* @internal
*/
function resolvePreset(
function resolvePresetInternal(
preset: GraphileConfig.Preset,
seenPluginNames: Set<string>,
depth: number,
): GraphileConfig.ResolvedPreset {
if (!isGraphileConfigPreset(preset)) {
throw new Error(
Expand Down Expand Up @@ -197,28 +227,31 @@ function resolvePreset(
);
}
}
const { extends: presets = [] } = preset;
const basePreset = resolvePresets(presets, false);
mergePreset(basePreset, preset);
} catch (e) {
throw new Error(
`Error occurred when resolving preset:\n ${String(e).replace(
/\n/g,
"\n ",
)}\nPreset: ${inspect(preset)}`,
);
}

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];
}
const { extends: presets = [], ...rest } = preset;
const basePreset = resolvePresetsInternal(
presets,
seenPluginNames,
depth + 1,
);

try {
mergePreset(basePreset, rest, seenPluginNames, depth);
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)}`,
);
}
}
Expand All @@ -233,24 +266,56 @@ function resolvePreset(
*/
function mergePreset(
targetPreset: GraphileConfig.ResolvedPreset,
sourcePreset: GraphileConfig.Preset,
sourcePreset: GraphileConfig.ResolvedPreset,
seenPluginNames: Set<string>,
_depth: number,
): void {
const sourcePluginNames: string[] = [];
if (sourcePreset.plugins) {
for (const plugin of sourcePreset.plugins) {
assertPlugin(plugin);
seenPluginNames.add(plugin.name);
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 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 ?? []),
]),
];
targetPreset.disablePlugins = 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])];
Expand Down

0 comments on commit 374f846

Please sign in to comment.