Skip to content
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

WIP: Postgraphile v5 Support (#58) #60

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Conversation

FelixZY
Copy link

@FelixZY FelixZY commented Jun 6, 2024

Description

This PR aims to migrate the plugin to Postgraphile v5.

#58

Performance impact

unknown

Security impact

unknown

Checklist

  • My code matches the project's code style and yarn lint:fix passes.
  • I've added tests for the new feature, and yarn test passes.
  • I have detailed the new feature in the relevant documentation.
  • I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
  • If this is a breaking change I've explained why.

After bumping to the v5 package versions, tests would fail with the
following error:

```
● Test suite failed to run

  ReferenceError: TextEncoder is not defined

  > 1 | import * as pg from "pg";
      | ^
    2 |
    3 | export async function withPgPool<T = any>(
    4 |   cb: (pool: pg.Pool) => Promise<T>

    at Object.<anonymous> (node_modules/pg/lib/crypto/utils-webcrypto.js:22:21)
    at Object.<anonymous> (node_modules/pg/lib/crypto/utils.js:8:20)
    at Object.<anonymous> (node_modules/pg/lib/crypto/sasl.js:2:16)
    at Object.<anonymous> (node_modules/pg/lib/client.js:5:12)
    at Object.<anonymous> (node_modules/pg/lib/index.js:3:14)
    at Object.<anonymous> (__tests__/helpers.ts:1:1)
    at Object.<anonymous> (__tests__/schema.minimal_type.test.ts:2:1)
```

Based on [information from @SimenB](
  jsdom/jsdom#2524 (comment)
), it seems like you probably should not be using
`jest-environment-jsdom` to start with if you need access to
`TextEncoder` or `TextDecoder`.

Based on [this](
  https://stackoverflow.com/a/72369912/1137077
) answer on SO, I was able to verify that `@jest-environment node` at
the top of test files fixed the issue. However, it would seem more
logical to apply this as a global setting, given that postgraphile is
meant to run in a node context.

After setting `testEnvironment: jest-environment-node` in the global
config, I found that tests again started failing. However, the new
failures seem related to the v5 changes to plugins which is expected at
this stage.
The previous `moduleResolution: node` setting prevent importing types
from `graphile-build-pg/pg-introspection`.

`NodeNext` was chosen based on
https://github.com/graphile/crystal/blob/91e87ab6516490a4cc7b7fc6400efb7623fbd331/graphile-build/graphile-build/tsconfig.json#L9
This seems to be better in line with other v5 plugins and the new
`GraphileConfig.Preset` type.
@FelixZY
Copy link
Author

FelixZY commented Jun 6, 2024

I'm still quite new to Postgraphile and have limited experience of either v4 or v5 plugins. Please let me know if I'm doing something strange or unexpected!


I'm finding a few things hard to understand and am unsure about my direction. @benjie, would you mind helping me out with some feedback?

The biggest issue right now is PostgisRegisterTypesPlugin.ts. My understanding is that i need to do something like the following:

declare global {
  namespace GraphileBuild {
    interface ScopeObject {
      isPgGISType?: boolean;
    }
  }
}

export default makeExtendSchemaPlugin((build) => {
  const asGeoJson =
    // HELP: "Property 'pgRegistry' does not exist on type 'BuildInput'" - not sure what the problem is
    build.input.pgRegistry.pgResources[
      // HELP: Do I need to use build.inflection here?
      // The old code uses
      // `sql.identifier(POSTGIS.namespaceName || "public", "st_asgeojson")`
      build.inflection.builtin("st_asgeojson")
    ];

  return {
    typeDefs: gql`
      # I've extracted these from the GeoJSON RFC.
      # Their final shape is still a WIP (I also want to reduce the risk of breakage)
      interface GeoJsonObject {
        geojson: String!
        type: String
        bbox: [Float!]
        srid: Int!
      }
      interface GeometryObject implements GeoJsonObject {
        type: String!
      }
      scalar Position
      type PointObject implements GeometryObject {
        coordinates: Position
      }
      ...
    `,
    plans: {
      GeoJsonObject: {
        geojson: {
          plan() {
            return asGeoJson
              .execute(
                /*
                * HELP: No idea what to put in here!
                * I would assume some sort of reference to
                * e.g. `$parentPlan: ExecutableStep` but I'm
                * not sure how to access that in this scope
                * (I noticed it could be accessed from
                * `build.registerObjectType` when looking at
                * https://github.com/graphile/crystal/blob/main/graphile-build/graphile-build-pg/src/plugins/PgTypesPlugin.ts#L378
                * ).
                */
              );
          },
        },
      },
      GeometryObject: {
        /* ... */
      },
      Position: GraphQlPositionScalar,
      PointObject: {
        /* ... */
      },
      // ...
    },
  };
}, "PostgisRegisterTypesPlugin");

(I have marked areas where I could use some specific help with HELP:)

Is this in the right ballpark? Could you point me to some other plugins that make use of SQL function calls to aid me as reference material?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants