feat: initial Graphile v5 migration - minimal baseline#669
Draft
pyramation wants to merge 79 commits intomainfrom
Draft
feat: initial Graphile v5 migration - minimal baseline#669pyramation wants to merge 79 commits intomainfrom
pyramation wants to merge 79 commits intomainfrom
Conversation
This commit establishes the foundation for migrating from Graphile v4 to v5: Core changes: - Update graphql from 15.10.1 to ^16.9.0 - Update postgraphile from ^4.14.1 to ^5.0.0-rc.4 - Update graphile-build from ^4.14.1 to ^5.0.0-rc.3 - Add new v5 dependencies: grafast, grafserv, graphile-config, pg-sql2 Architecture changes: - Rewrite graphile-settings to use v5 preset pattern (PostGraphileAmberPreset) - Update graphile-cache for v5 instance structure (pgl, serv, handler) - Rewrite graphql/server middleware to use v5 postgraphile() and grafserv - Update graphql/types with GraphileConfig.Preset pattern - Use node16 moduleResolution for v5 ESM exports Temporarily disabled packages (to be migrated incrementally): - graphql/explorer - graphql/codegen - graphql/test - graphql/react - graphile/graphile-test - packages/cli The server middleware now uses: - v5 preset-based configuration - grafserv for Express integration - grafast context for request context (replacing pgSettings function) - Direct SQL queries in api.ts (replacing graphile-query) Next steps documented in docs/plan/graphile-v5-migration.md
Temporarily disabled in CI workflow: - All graphile/* plugin tests (depend on v4 APIs) - All graphql/* package tests (depend on v4 server) - packages/cli and jobs/knative-job-service (depend on graphql packages) Also disabled builds for: - graphql/playwright-test - graphql/server-test These will be re-enabled incrementally as each package is migrated to v5.
This allows PRs targeting develop-v5 to run CI tests, making it suitable as a long-lived development branch for the v5 migration.
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:
|
All tests are temporarily disabled on the develop-v5 branch. They will be re-enabled when v5 is ready to merge into main.
This is a test PR to verify that PRs targeting develop-v5 run CI correctly.
These tests depend on graphile-test and use v4 API patterns. They will be rewritten for v5 as part of the migration.
…rver-ci test: re-enable graphql/server test in CI
upgrade related graphile plugins removed some graphile plugins for now
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.
…-v5-migration feat: migrate codegen to PostGraphile v5 / GraphQL 16
- Rewrite src/index.ts to use v5 APIs: - Replace createPostGraphileSchema with makeSchema from graphile-build - Replace withPostGraphileContext with execute from grafast - Use withPgClientFromPgService for proper database connection handling - Update GraphileSettings interface to use GraphileConfig.Preset - Update package.json dependencies to v5 versions: - graphql ^16.9.0 - postgraphile ^5.0.0-rc.4 - grafast ^1.0.0-rc.4 - graphile-build ^5.0.0-rc.3 - graphile-build-pg ^5.0.0-rc.3 - graphile-config 1.0.0-rc.3 - Bump version to 3.0.0 (major version for v5 migration) - Update tsconfig to use node16 module resolution - Add comprehensive test suite with 9 passing tests
- Add graphile-authz: authorization plugin with declarative rules and SQL generation - Add postgraphile-plugin-pgvector: vector similarity search plugin - Delete graphile-plugin-connection-filter (use community version if needed) - Delete graphile-simple-inflector (inlined in graphile-settings) - Enable both new plugins in GitHub workflow
…nflector - graphile-plugin-connection-filter: use community version postgraphile-plugin-connection-filter@^3.0.0-rc.1 if needed - graphile-simple-inflector: already inlined in graphile-settings as CustomInflectorPlugin
- Add jest.config.js for graphile-authz (66 tests passing) - Add jest.config.js for postgraphile-plugin-pgvector - Fix pgvector test imports (use pgsql-test for PgTestClient) - Add pgsql-test as devDependency for pgvector Note: pgvector tests fail due to plugin using traditional resolver instead of Grafast steps - needs conversion to v5 step-based API
feat: upgrade graphile-query to PostGraphile v5 / GraphQL 16
- Add RLS_MODULE_SQL query to fetch RLS module data with private schema name - Add api_id to DOMAIN_LOOKUP_SQL and API_NAME_LOOKUP_SQL queries - Add RlsModuleRow interface for type safety - Add queryRlsModule function to fetch RLS module by API ID - Add toRlsModule helper to convert database row to RlsModule interface - Update toApiStructure to accept and include RLS module data - Update resolveApiNameHeader and resolveDomainLookup to fetch RLS module This enables the authentication middleware (auth.ts) to access the rlsModule data (authenticate, authenticateStrict, privateSchema) which is required for PostGraphile v5 authentication flow.
…e-release error PostGraphile v5's pgl.release() internally calls serv.release() on the grafserv instance. Calling serv.release() explicitly before pgl.release() causes a 'Release has already been called' error since grafserv throws when release() is called twice. The fix removes the explicit serv.release() call and lets pgl.release() handle the grafserv cleanup internally.
…Graphile v5 The grafast.context callback receives a RequestContext object, not a generic context. In grafserv/express/v4, the Express request is available at requestContext.expressv4.req, not ctx.node.req. This was preventing the authentication middleware from properly passing the token to the GraphQL context, causing bearer token authentication to fail silently (always using anonRole instead of roleName).
- Replace traditional resolve function with Grafast plan function - Use lambda step with grafastContext() to access withPgClient - Use object step to combine field arguments - All 9 pgvector tests now pass
…ture The grafast.context callback expects Partial<Grafast.RequestContext>, not the full Grafast.RequestContext type.
…e-plugins feat: add graphile-authz and postgraphile-plugin-pgvector packages
Added INFO-level logging throughout the auth middleware to help debug authentication issues: - Log when middleware is called and whether api is present - Log RLS module details (authenticate, authenticateStrict, privateSchema) - Log authFn selection and strictAuth setting - Log authorization header parsing - Log the actual auth query being executed - Log query results and success/failure - Log when skipping auth due to missing config
The RLS module table is in metaschema_modules_public.rls_module, not services_public.rls_module. This was causing the RLS module query to return null, which made authentication skip entirely.
Simplified query functions to just call pool.query() directly without try/catch blocks that silently swallow 'does not exist' errors. Errors should propagate so issues are visible, not hidden.
…ication-upgrade feat(api): add RLS module support to API resolution for PostGraphile v5
- Add complete plugin inventory section tracking all plugins from main - Document plugins inlined into graphile-settings (9 presets) - Document plugins using community packages (connection-filter, many-to-many) - Document standalone packages upgraded to v5 (6 packages) - Document packages deleted (simple-inflector, connection-filter) - Document packages requiring v5 port (8 packages) - Update implementation checklist with detailed plugin status
…e-release fix(graphile-cache): remove redundant serv.release() to prevent double-release error
…migration-status docs: update plugin migration status in v5 migration plan
- Add PgTypeMappingsPlugin that maps custom PostgreSQL types to GraphQL scalars - Default mappings: email, hostname, url, origin -> String; multiple_select, single_select -> JSON - Uses v5 gather hooks to create codecs for custom types - Uses v5 schema hooks to map codecs to GraphQL types - Add PgTypeMappingsPreset to ConstructivePreset - Export plugin and types from plugins/index.ts
- Remove single_select and multiple_select type mappings (jsonb types) - Simplify TypeMapping interface to only support String type - Remove JSON-related code from fromPg function - Keep only email, hostname, origin, url mappings
… rename to constructiveInternalType* - Remove single_select and multiple_select domain files from fixtures - Rename pgpmInternalType* to constructiveInternalType* in all domain comments - Update pgpm.plan to remove single_select/multiple_select entries
- email: remove complex regex, keep as citext - hostname: remove regex - url: remove regex - origin: remove regex - attachment: remove regex - image: jsonb requiring only 'url' key - upload: jsonb requiring at least one of 'url', 'id', or 'key'
URL validation: ^https?://[^\s]+$ - Must start with http:// or https:// - Must have at least one character after protocol - No whitespace allowed
feat: inline pg-type-mappings plugin into graphile-settings
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
This PR establishes the foundation for migrating from PostGraphile v4 to v5, including the GraphQL 15→16 upgrade. The approach follows an incremental strategy: disable all plugins and middleware initially to get a minimal working baseline, then re-enable features incrementally in follow-up PRs.
Core dependency updates:
graphql: 15.10.1 → ^16.9.0postgraphile: ^4.14.1 → ^5.0.0-rc.4graphile-build: ^4.14.1 → ^5.0.0-rc.3grafast,grafserv,graphile-config,pg-sql2Architecture changes:
graphile-settingsrewritten to use v5 preset pattern (PostGraphileAmberPreset)graphql/server/middleware/graphile.tsrewritten to usepostgraphile()+grafservfor Express integrationgraphile-cacheupdated for v5 instance structurenode16moduleResolution (required for v5 ESM exports)Temporarily disabled packages (to be migrated incrementally):
graphql/explorer,graphql/codegen,graphql/test,graphql/reactgraphql/playwright-test,graphql/server-testgraphile/graphile-test,packages/cliCI workflow changes:
develop-v5branch to workflow triggers for long-lived developmentMigration plan documented in
docs/plan/graphile-v5-migration.md.Review & Testing Checklist for Human
/graphiqlif available.graphql/server/src/middleware/graphile.tslines 52-53: The context function uses(ctx: unknown)with type assertions - verify this matches the actual grafserv context structure at runtime.graphile-settings/src/index.ts: This was rewritten from v4 options to v5 preset pattern - verify the minimal preset configuration is correct.Recommended test plan:
pnpm install && pnpm build(should pass)/graphiqland run a basic queryNotes
The v5 spike repo (constructive-io/graphile) was used as reference for correct patterns but not imported directly.
Link to Devin run: https://app.devin.ai/sessions/0b3cd9962e044fd28cc3cefb5fb61ea4
Requested by: Dan Lynch (@pyramation)