-
Notifications
You must be signed in to change notification settings - Fork 5
feat: initial Graphile v5 migration - minimal baseline #669
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
Draft
pyramation
wants to merge
9
commits into
main
Choose a base branch
from
develop-v5
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
Contributor
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
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.
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)