Make PgServiceConfiguration.adaptor an adaptor instance, not path (#1985

)
graphile · Jun 5, 2024 · 73fd6f0 · 73fd6f0
2 parents 8062530 + 1b7395c
commit 73fd6f0
Show file tree

Hide file tree

Showing 14 changed files with 187 additions and 160 deletions.
diff --git a/.changeset/khaki-zoos-bathe.md b/.changeset/khaki-zoos-bathe.md
@@ -0,0 +1,21 @@
+---
+"graphile-build-pg": patch
+"postgraphile": patch
+"@dataplan/pg": patch
+---
+
+🚨 PostgreSQL adaptor is no longer loaded via string value; instead you must
+pass the adaptor instance directly. If you have
+`adaptor: "@dataplan/pg/adaptors/pg"` then replace it with
+`adaptor: await import("@dataplan/pg/adaptors/pg")`. (This shouldn't cause you
+issues because you _should_ be using `makePgService` to construct your
+`pgServices` rather than building raw objects.)
+
+🚨 If you've implemented a custom PgAdaptor, talk to Benjie about how to port
+it. (Should be straightforward, but no point me figuring it out if no-one has
+done it yet 🤷)
+
+This change improves bundle-ability by reducing the number of dynamic imports.
+
+Also: `PgAdaptorOptions` has been renamed to `PgAdaptorSettings`, so please do a
+global find and replace for that.
diff --git a/grafast/dataplan-pg/src/adaptors/pg.ts b/grafast/dataplan-pg/src/adaptors/pg.ts
@@ -28,6 +28,27 @@ import type {
 import type { MakePgServiceOptions } from "../interfaces.js";
 import type { PgAdaptor } from "../pgServices.js";
 
+declare global {
+  namespace Grafast {
+    interface Context {
+      pgSettings: {
+        [key: string]: string;
+      } | null;
+      withPgClient: WithPgClient<NodePostgresPgClient>;
+      pgSubscriber: PgSubscriber | null;
+    }
+  }
+  namespace GraphileConfig {
+    interface PgAdaptors {
+      "@dataplan/pg/adaptors/pg": {
+        adaptorSettings: PgAdaptorSettings | undefined;
+        makePgServiceOptions: PgAdaptorMakePgServiceOptions;
+        client: NodePostgresPgClient;
+      };
+    }
+  }
+}
+
 // Set `DATAPLAN_PG_PREPARED_STATEMENT_CACHE_SIZE=0` to disable prepared statements
 const cacheSizeFromEnv = process.env.DATAPLAN_PG_PREPARED_STATEMENT_CACHE_SIZE
   ? parseInt(process.env.DATAPLAN_PG_PREPARED_STATEMENT_CACHE_SIZE, 10)
@@ -354,7 +375,7 @@ export function makeWithPgClientViaPgClientAlreadyInTransaction(
   return withPgClient;
 }
 
-export interface PgAdaptorOptions {
+export interface PgAdaptorSettings {
   /** ONLY FOR USE IN TESTS! */
   poolClient?: pg.PoolClient;
   /** ONLY FOR USE IN TESTS! */
@@ -374,8 +395,11 @@ export interface PgAdaptorOptions {
   superuserConnectionString?: string;
 }
 
+/** @deprecated Use PgAdaptorSettings instead. */
+export type PgAdaptorOptions = PgAdaptorSettings;
+
 export function createWithPgClient(
-  options: PgAdaptorOptions = Object.create(null),
+  options: PgAdaptorSettings = Object.create(null),
   variant?: "SUPERUSER" | string | null,
 ): WithPgClient<NodePostgresPgClient> {
   if (variant === "SUPERUSER") {
@@ -414,8 +438,10 @@ export function createWithPgClient(
 }
 
 // This is here as a TypeScript assertion, to ensure we conform to PgAdaptor
-const _testValidAdaptor: PgAdaptor<"@dataplan/pg/adaptors/pg">["createWithPgClient"] =
-  createWithPgClient;
+const adaptor: PgAdaptor<"@dataplan/pg/adaptors/pg"> = {
+  createWithPgClient,
+  makePgService,
+};
 
 const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
 
@@ -702,21 +728,13 @@ export class PgSubscriber<
   }
 }
 
-declare global {
-  namespace Grafast {
-    interface Context {
-      pgSettings: {
-        [key: string]: string;
-      } | null;
-      withPgClient: WithPgClient<NodePostgresPgClient>;
-      pgSubscriber: PgSubscriber | null;
-    }
-  }
+export interface PgAdaptorMakePgServiceOptions extends MakePgServiceOptions {
+  pool?: pg.Pool;
 }
 
 export function makePgService(
-  options: MakePgServiceOptions & { pool?: pg.Pool },
-): GraphileConfig.PgServiceConfiguration {
+  options: PgAdaptorMakePgServiceOptions,
+): GraphileConfig.PgServiceConfiguration<"@dataplan/pg/adaptors/pg"> {
   const {
     name = "main",
     connectionString,
@@ -757,26 +775,27 @@ export function makePgService(
     pgSubscriber = new PgSubscriber(pool);
     releasers.push(() => pgSubscriber!.release?.());
   }
-  const service: GraphileConfig.PgServiceConfiguration = {
-    name,
-    schemas: Array.isArray(schemas) ? schemas : [schemas ?? "public"],
-    withPgClientKey: withPgClientKey as any,
-    pgSettingsKey: pgSettingsKey as any,
-    pgSubscriberKey: pgSubscriberKey as any,
-    pgSettings,
-    pgSettingsForIntrospection,
-    pgSubscriber,
-    adaptor: "@dataplan/pg/adaptors/pg",
-    adaptorSettings: {
-      pool,
-      superuserConnectionString,
-    },
-    async release() {
-      // Release in reverse order
-      for (const releaser of [...releasers].reverse()) {
-        await releaser();
-      }
-    },
-  };
+  const service: GraphileConfig.PgServiceConfiguration<"@dataplan/pg/adaptors/pg"> =
+    {
+      name,
+      schemas: Array.isArray(schemas) ? schemas : [schemas ?? "public"],
+      withPgClientKey: withPgClientKey as any,
+      pgSettingsKey: pgSettingsKey as any,
+      pgSubscriberKey: pgSubscriberKey as any,
+      pgSettings,
+      pgSettingsForIntrospection,
+      pgSubscriber,
+      adaptor,
+      adaptorSettings: {
+        pool,
+        superuserConnectionString,
+      },
+      async release() {
+        // Release in reverse order
+        for (const releaser of [...releasers].reverse()) {
+          await releaser();
+        }
+      },
+    };
   return service;
 }
diff --git a/grafast/dataplan-pg/src/executor.ts b/grafast/dataplan-pg/src/executor.ts
@@ -73,7 +73,7 @@ export interface PgClientResult<TData> {
  */
 export interface PgClient {
   query<TData>(opts: PgClientQuery): Promise<PgClientResult<TData>>;
-  withTransaction<T>(callback: (client: PgClient) => Promise<T>): Promise<T>;
+  withTransaction<T>(callback: (client: this) => Promise<T>): Promise<T>;
 }
 
 export interface WithPgClient<TPgClient extends PgClient = PgClient> {

diff --git a/grafast/dataplan-pg/src/index.ts b/grafast/dataplan-pg/src/index.ts
@@ -1,7 +1,6 @@
 import type { GrafastSubscriber } from "grafast";
 import { exportAsMany } from "grafast";
 
-import type { PgAdaptorOptions } from "./adaptors/pg.js";
 import {
   domainOfCodec,
   enumCodec,
@@ -429,16 +428,19 @@ declare global {
   namespace GraphileConfig {
     interface PgServiceConfiguration<
       TAdaptor extends
-        keyof GraphileConfig.PgDatabaseAdaptorOptions = keyof GraphileConfig.PgDatabaseAdaptorOptions,
+        keyof GraphileConfig.PgAdaptors = keyof GraphileConfig.PgAdaptors,
     > {
       name: string;
       schemas?: string[];
 
-      adaptor: TAdaptor;
-      adaptorSettings?: GraphileConfig.PgDatabaseAdaptorOptions[TAdaptor];
+      adaptor: PgAdaptor<TAdaptor>;
+      adaptorSettings?: GraphileConfig.PgAdaptors[TAdaptor]["adaptorSettings"];
 
       /** The key on 'context' where the withPgClient function will be sourced */
-      withPgClientKey: KeysOfType<Grafast.Context & object, WithPgClient>;
+      withPgClientKey: KeysOfType<
+        Grafast.Context & object,
+        WithPgClient<GraphileConfig.PgAdaptors[TAdaptor]["client"]>
+      >;
 
       /** Return settings to set in the session */
       pgSettings?: (
@@ -471,12 +473,26 @@ declare global {
     }
 
     interface Preset {
-      pgServices?: ReadonlyArray<PgServiceConfiguration>;
+      pgServices?: ReadonlyArray<
+        {
+          [Key in keyof GraphileConfig.PgAdaptors]: PgServiceConfiguration<Key>;
+        }[keyof GraphileConfig.PgAdaptors]
+      >;
     }
 
-    interface PgDatabaseAdaptorOptions {
-      "@dataplan/pg/adaptors/pg": PgAdaptorOptions;
-      /* Add your own via declaration merging */
+    interface PgAdaptors {
+      /*
+       * Add your adaptor configurations via declaration merging; they should
+       * conform to this:
+       *
+       * ```
+       * [moduleName: string]: {
+       *   adaptorSettings: { ... } | undefined;
+       *   makePgServiceOptions: MakePgServiceOptions & { ... };
+       *   client: PgClient & MyPgClientStuff;
+       * };
+       * ```
+       */
     }
   }
   namespace DataplanPg {

diff --git a/grafast/dataplan-pg/src/pgServices.ts b/grafast/dataplan-pg/src/pgServices.ts
@@ -1,18 +1,21 @@
-import { pathToFileURL } from "node:url";
-
-import type { PgClient, WithPgClient } from "./executor.ts";
+import type { PgClient, WithPgClient } from "./executor.js";
 
 type PromiseOrDirect<T> = T | PromiseLike<T>;
 
 /** @experimental */
 export interface PgAdaptor<
   TAdaptor extends
-    keyof GraphileConfig.PgDatabaseAdaptorOptions = keyof GraphileConfig.PgDatabaseAdaptorOptions,
+    keyof GraphileConfig.PgAdaptors = keyof GraphileConfig.PgAdaptors,
 > {
   createWithPgClient: (
-    adaptorSettings: GraphileConfig.PgServiceConfiguration<TAdaptor>["adaptorSettings"],
-    variant?: "SUPERUSER" | null,
-  ) => PromiseOrDirect<WithPgClient>;
+    adaptorSettings: GraphileConfig.PgAdaptors[TAdaptor]["adaptorSettings"],
+    variant?: "SUPERUSER" | string | null,
+  ) => PromiseOrDirect<
+    WithPgClient<GraphileConfig.PgAdaptors[TAdaptor]["client"]>
+  >;
+  makePgService: (
+    options: GraphileConfig.PgAdaptors[TAdaptor]["makePgServiceOptions"],
+  ) => GraphileConfig.PgServiceConfiguration<TAdaptor>;
 }
 
 /**
@@ -26,83 +29,43 @@ export function isPromiseLike<T>(
 
 const isTest = process.env.NODE_ENV === "test";
 
-interface PgClientBySourceCacheValue {
-  withPgClient: WithPgClient;
+interface PgClientBySourceCacheValue<TPgClient extends PgClient = PgClient> {
+  withPgClient: WithPgClient<TPgClient>;
   retainers: number;
 }
 
 const withPgClientDetailsByConfigCache = new Map<
-  GraphileConfig.PgServiceConfiguration,
+  GraphileConfig.PgServiceConfiguration<any>,
   PromiseOrDirect<PgClientBySourceCacheValue>
 >();
 
-function reallyLoadAdaptor<
-  TAdaptor extends
-    keyof GraphileConfig.PgDatabaseAdaptorOptions = keyof GraphileConfig.PgDatabaseAdaptorOptions,
->(adaptorString: TAdaptor): PromiseOrDirect<PgAdaptor<TAdaptor>> {
-  try {
-    const adaptor = require(adaptorString);
-    return adaptor?.createWithPgClient ? adaptor : adaptor?.default;
-  } catch (e) {
-    if (e.code === "ERR_REQUIRE_ESM") {
-      const importSpecifier = adaptorString.match(/^([a-z]:|\.\/|\/)/i)
-        ? pathToFileURL(adaptorString).href
-        : adaptorString;
-      const adaptorPromise = import(importSpecifier);
-      return adaptorPromise.then((adaptor) =>
-        adaptor?.createWithPgClient ? adaptor : adaptor?.default,
-      );
-    } else {
-      throw e;
-    }
-  }
-}
-
-const loadAdaptorCache = new Map<string, PromiseOrDirect<PgAdaptor<any>>>();
-function loadAdaptor<
-  TAdaptor extends
-    keyof GraphileConfig.PgDatabaseAdaptorOptions = keyof GraphileConfig.PgDatabaseAdaptorOptions,
->(adaptorString: TAdaptor): PromiseOrDirect<PgAdaptor<TAdaptor>> {
-  const cached = loadAdaptorCache.get(adaptorString);
-  if (cached) {
-    return cached;
-  } else {
-    const result = reallyLoadAdaptor(adaptorString);
-    loadAdaptorCache.set(adaptorString, result);
-    if (isPromiseLike(result)) {
-      result.then(
-        (resolved) => {
-          loadAdaptorCache.set(adaptorString, resolved);
-        },
-        () => {},
-      );
-    }
-    return result;
-  }
-}
-
 /**
  * Get or build the 'withPgClient' callback function for a given database
  * config, caching it to make future lookups faster.
  */
-export function getWithPgClientFromPgService(
-  config: GraphileConfig.PgServiceConfiguration,
-): PromiseOrDirect<WithPgClient> {
+export function getWithPgClientFromPgService<
+  TAdaptor extends
+    keyof GraphileConfig.PgAdaptors = keyof GraphileConfig.PgAdaptors,
+>(
+  config: GraphileConfig.PgServiceConfiguration<TAdaptor>,
+): PromiseOrDirect<
+  WithPgClient<GraphileConfig.PgAdaptors[TAdaptor]["client"]>
+> {
+  type TPgClient = GraphileConfig.PgAdaptors[TAdaptor]["client"];
   const existing = withPgClientDetailsByConfigCache.get(config);
   if (existing) {
     if (isPromiseLike(existing)) {
       return existing.then((v) => {
         v.retainers++;
-        return v.withPgClient;
+        return v.withPgClient as WithPgClient<TPgClient>;
       });
     } else {
       existing.retainers++;
-      return existing.withPgClient;
+      return existing.withPgClient as WithPgClient<TPgClient>;
     }
   } else {
     const promise = (async () => {
-      const adaptor = await loadAdaptor(config.adaptor);
-      const factory = adaptor?.createWithPgClient;
+      const factory = config.adaptor?.createWithPgClient;
       if (typeof factory !== "function") {
         throw new Error(
           `'${config.adaptor}' does not look like a withPgClient adaptor - please ensure it exports a method called 'createWithPgClient'`,
@@ -144,7 +107,7 @@ export function getWithPgClientFromPgService(
     promise.catch(() => {
       withPgClientDetailsByConfigCache.delete(config);
     });
-    return promise.then((v) => v.withPgClient);
+    return promise.then((v) => v.withPgClient as WithPgClient<TPgClient>);
   }
 }
 
@@ -170,8 +133,7 @@ export async function withSuperuserPgClientFromPgService<T>(
   pgSettings: { [key: string]: string } | null,
   callback: (client: PgClient) => T | Promise<T>,
 ): Promise<T> {
-  const adaptor = await loadAdaptor(config.adaptor);
-  const withPgClient = await adaptor.createWithPgClient(
+  const withPgClient = await config.adaptor.createWithPgClient(
     config.adaptorSettings,
     "SUPERUSER",
   );