-
Notifications
You must be signed in to change notification settings - Fork 5
feat: migrate codegen to PostGraphile v5 / GraphQL 16 #680
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
upgrade related graphile plugins removed some graphile plugins for now
- Re-enable codegen build scripts that were disabled during v5 migration - Fix GraphQL 16 compatibility in gql-ast: export Kind and OperationTypeNode - Fix codegen to use OperationTypeNode enum instead of string literals - Fix codegen to use Kind enum values instead of string literals - Remove pgSettings option from buildSchemaSDL calls (v5 uses preset config) - Re-enable schema export from graphql-server for codegen consumption All 190 codegen tests pass.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Replace PostGraphile/Grafast machinery with direct SQL queries - Remove gql.ts and codegen/orm directory (no longer needed) - Add clean switch-based routing for different resolution modes: - services-disabled: fast path when services API is off - schemata-header: handle X-Schemata header - api-name-header: handle X-Api-Name header - meta-schema-header: handle X-Meta-Schema header - domain-lookup: domain-based API resolution - Reduce complexity from ~770 lines to ~400 lines - Maintain same caching behavior and error handling
…ove unnecessary exports
…, and hover effects
- Create test SQL schema with 5 tables (users, posts, tags, post_tags, comments) - Include many-to-many relationships, unique constraints, foreign keys - Add schema snapshot test using buildSchemaSDL - Snapshot captures how plugins sculpt the API (inflection, filters, etc.)
- Add _manyToManyRelation inflector override to InflektPlugin - Simplifies verbose names like 'tagsByPostTagPostIdAndTagId' to 'tags' - Falls back to verbose naming if: - Smart tag override exists (manyToManyFieldName) - Direct relation to same target table exists (would conflict) - Multiple many-to-many relations to same target table - Update schema snapshot with simplified field names
…mentation - Document all presets and plugins with detailed explanations - Add Quick Start example for PostGraphile v5 - Document each feature: MinimalPreset, InflektPreset, Connection Filter, EnableAllFilterColumns, ManyToManyOptIn, NoUniqueLookup, MetaSchema, TsvectorCodec, ConflictDetector, InflectorLogger - Add Presets and Plugins reference tables - Add Smart Tags reference with SQL examples - Add Environment Variables section - Follow Constructive README formatting guidelines
The TsvectorCodecPlugin was creating codecs but not registering them with the GraphQL type system. Added schema.hooks.init to map tsvector and tsquery codecs to GraphQL String type using setGraphQLTypeForPgCodec. This fixes the error: 'Do not know how to convert PgCodec tsvector into a GraphQL type'
- Add Phase 6.1: Restore @constructive-io/graphql-test package - Document the difference between graphile-test and graphql-test - Add restoration plan using git checkout to preserve history - Add plugin test requirements (tsvector, many-to-many, filters, meta schema) - Update Phase 5 checklist with testing infrastructure tasks - Mark completed plugins in Phase 6 checklist
Restored graphql/test from main branch and updated for v5: - Updated dependencies to PostGraphile v5, GraphQL 16, grafast, graphile-build - Updated GraphQLTest to use ConstructivePreset from graphile-settings - Replaced createPostGraphileSchema with makeSchema from graphile-build - Updated tsconfig for node16 module resolution (required for v5 imports) This package wraps graphile-test and pre-loads ConstructivePreset for testing with the full Constructive plugin configuration.
- Enable graphql/test package in CI workflow matrix - Update plugin tests to use v5 preset API instead of deprecated graphile option - Convert v4-style builder.hook() plugins to v5 GraphileConfig.Plugin format - Use GraphQLObjectType_fields hook with proper context.Self check
The existing snapshots were created with PostGraphile v4 / GraphQL 15 and need to be regenerated for v5 / GraphQL 16. Key differences include: - New introspection fields (specifiedByURL, isOneOf) - Different schema structure from ConstructivePreset plugins CI will regenerate these snapshots on first run.
… snapshots - Fix TypeScript errors in plugin tests (add type assertions for data properties) - Fix TypeScript errors in graphile-tx test (add type assertions) - Remove invalid pgOmitListSuffix option (doesn't exist in v5) - Regenerate all snapshots for PostGraphile v5 / GraphQL 16 All 20 tests now pass locally.
The findAvailablePort() function had a race condition where it would: 1. Create a temp server to check if a port is available 2. Close the temp server 3. Return the port number Between steps 2 and 3, another process could grab the port, causing EADDRINUSE errors in CI when multiple test suites run. Using port 0 lets the OS atomically assign an available port when server.listen(0) is called, eliminating the race condition entirely.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: migrate codegen to PostGraphile v5 / GraphQL 16
Summary
This PR enables the codegen package to work with PostGraphile v5 and GraphQL 16 by fixing compatibility issues that were blocking the build.
Key changes:
gql-ast: now exportsKindandOperationTypeNodeenumsOperationTypeNode.QUERY/MUTATIONinstead of string literals'query'/'mutation'Kind.INLINE_FRAGMENT,Kind.FIELD, etc. instead of string literalspgSettingsoption frombuildSchemaSDLcalls (v5 uses preset configuration instead)All 190 codegen tests pass locally.
Updates since last revision
graphql/test package now enabled in CI and passing: Fixed TypeScript errors in plugin tests by adding type assertions for data properties. Removed invalid
pgOmitListSuffixoption (doesn't exist in v5). Regenerated all snapshots for PostGraphile v5 / GraphQL 16. All 20 tests now pass.Previous updates: Restored
@constructive-io/graphql-testpackage for v5, fixed TsvectorCodecPlugin to register GraphQL type mappings, updated migration plan document, updated graphile-settings README, simplified many-to-many field names with conflict detection, added schema snapshot test, modernized dev fallback page design, disabled error masking, refactored api.ts to use direct SQL queries, fixed GraphiQL route, added favicon middleware, and cleaned up error-handler.ts.Review & Testing Checklist for Human
tsvectorcolumn and verify it appears asStringtype in the GraphQL schema (not an error). The fix ingraphile/graphile-settings/src/plugins/tsvector-codec.tsiterates through all codecs.graphql/test/src/graphile-test.tswas significantly rewritten for v5. Verify theGraphQLTestfunction correctly extendsConstructivePresetand therunGraphQLInContextcall has the right parameters.graphql/test/__tests__/now use type assertions likeres.data as { testPluginField?: string }. Verify these don't mask real type issues.EADDRINUSE: address already in use 127.0.0.1:5556) appears to be a pre-existing port binding issue unrelated to these changes. The graphql/test package passes.maskErrorfunction returns all errors unmasked. Sensitive database errors will be visible to API clients in production.Recommended test plan:
tsvectorcolumn and verify it appears asStringin GraphQLNotes
develop-v5branchdocs/plan/graphile-v5-migration.mdgraphql/testpackage is now enabled in CI and all 20 tests passconflict-detector.tsfor accessing codecs viabuild.input.pgRegistry.pgCodecsLink to Devin run: https://app.devin.ai/sessions/ba45b64c92bb4307bf9e920123213d9e
Requested by: Dan Lynch (@pyramation)