-
Notifications
You must be signed in to change notification settings - Fork 5
refactor(graphql-codegen): separate concerns by moving business logic to core #650
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
Open
pyramation
wants to merge
21
commits into
main
Choose a base branch
from
devin/1769141729-codegen-separation-of-concerns
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.
+3,764
−3,058
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
… to core Move all business logic from cli/ to core/ directory structure: - Move cli/codegen/* to core/codegen/* (code generation) - Move cli/introspect/* to core/introspect/* (schema introspection) - Move cli/watch/* to core/watch/* (watch mode) - Move cli/commands/shared.ts to core/pipeline/ (codegen pipeline) - Create core/config/ with loader.ts and resolver.ts (config utilities) - Create core/output/ with writer.ts (file writing utilities) CLI commands are now thin wrappers that: - Handle user prompts and CLI arguments - Call core functions for business logic - Format and display output This improves separation of concerns and makes the core logic reusable outside of the CLI context. Update all test imports to reference new core/ paths. Update package.json copy:ts script for new file locations.
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:
|
… CLI Phase 2 of separation of concerns: - Add @constructive-io/graphql-server as dependency to graphql-codegen - Remove @constructive-io/graphql-codegen devDependency from graphql-server (generated code is committed) - Create core/database module with buildSchemaFromDatabase function - Clean up packages/cli codegen.ts to use inquirerer pattern and import from graphql-codegen - Export buildSchemaFromDatabase and buildSchemaSDLFromDatabase from graphql-codegen This breaks the circular dependency by making it a one-way dependency: graphql-codegen -> graphql-server (no reverse dependency)
The codegen command now uses prompter.prompt() for input handling, so tests need to provide a mock prompter that returns the argv values.
…ements - Rename 'generate' to 'react-query' for generator type (more specific naming) - Rename 'generate-orm' to 'orm' for generator type - Replace ternary operators with switch statements that throw on unknown cases - Improves code clarity and type safety
…gen command - Convert orm boolean check to explicit generatorType variable - Use switch statement with 'orm' and 'react-query' cases - Add throw for unknown generator type (defensive programming) - Extract output directory calculation to separate variable
Extend generateReactQuery and generateOrm to accept database options directly,
eliminating the need to call buildSchemaFromDatabase first.
New usage:
await generateReactQuery({ database: 'mydb', schemas: ['public'] });
await generateOrm({ database: 'mydb', schemas: ['public'] });
Changes:
- Add DatabaseSchemaSource class for in-memory database introspection
- Update createSchemaSource to detect and handle database mode
- Update ConfigOverrideOptions to include database and schemas options
- Update generateReactQuery and generateOrm to pass database options through
- Add validateSourceOptions to check for exactly one source type
- Add ephemeral database utilities to pgsql-client (createEphemeralDb) - Add PgpmModuleSchemaSource class for introspecting PGPM modules - Support two input modes: pgpmModulePath and pgpmWorkspacePath + pgpmModuleName - Add keepDb option for debugging ephemeral databases - Update config resolver and generate commands to handle PGPM options - Add @pgpmjs/core, pgsql-client, and pgsql-seed dependencies to graphql-codegen
…overy - Add apiNames option to schema sources (mutually exclusive with schemas) - Create api-schemas.ts with utilities for resolving schemas from API names - Query services_public.api_schemas joined with services_public.apis and metaschema_public.schema - Validate services_public/metaschema_public schemas exist: - For database mode: validate at the beginning - For PGPM modes: validate after module deployment - Update config resolver to handle apiNames in ConfigOverrideOptions - Update generate.ts and generate-orm.ts to extract and pass apiNames - Add pg-cache and pg-env as dependencies
…y and --orm flags - Create unified generate() function that respects config.reactQuery.enabled and orm.enabled - Simplify CLI to use single 'generate' command with --reactquery and --orm flags - Remove old 'generate-orm' command (now unified into 'generate') - Add CLI support for new source options: --database, --pgpm-module-path, --pgpm-workspace-path, --pgpm-module-name - Add CLI support for schema options: --schemas, --api-names - Update help text with comprehensive documentation - Update README with new CLI usage examples - Update package.json example scripts
…ctQuery/generateOrm private - Create core/codegen/shared/ module for shared types generation - Update unified generate() to use shared types when both React Query and ORM are enabled - Add sharedTypesPath option to React Query SDK and ORM generators - Make generateReactQuery() and generateOrm() internal (not exported from package) - Update packages/cli to use unified generate() function - Export only generate() as the public API
…nd use Babel AST for barrel exports - Add database, pgpmModulePath, pgpmWorkspacePath, pgpmModuleName, schemas, apiNames, keepDb fields to GraphQLSDKConfigTarget and ResolvedConfig types - Remove all (config as any) type casts now that fields are first-class citizens - Simplify buildTargetOverrides by removing redundant undefined assignments (validation happens in loadAndResolveConfig) - Update generateSharedBarrel in shared/index.ts to use Babel AST instead of string concatenation - Update generateUnifiedBarrel in generate-unified.ts to use Babel AST instead of string concatenation - Update resolveMultiTargetConfig to check for all source types (database, pgpm, etc.)
…ttern - Delete ResolvedConfig, ResolvedWatchConfig, ResolvedQueryKeyConfig types - Use GraphQLSDKConfigTarget everywhere (single type with optional fields) - Delete buildTargetOverrides function - use destructuring inline - Create getConfigOptions function following pgpm/env pattern - Make ConfigOverrideOptions extend GraphQLSDKConfigTarget - Rename ResolvedTargetConfig to TargetConfig - Update all imports and usages to use new types - Remove (config as any) casts since config now has all fields - Rename reactQuery/orm options to enableReactQuery/enableOrm in GenerateOptions - Update tests to use QueryKeyConfig instead of ResolvedQueryKeyConfig
…ndpoints during introspection This fixes DNS resolution issues on macOS where subdomains like api.localhost don't resolve automatically to 127.0.0.1 (unlike browsers which handle *.localhost). Uses undici Agent with custom DNS lookup to resolve *.localhost to 127.0.0.1 at codegen time only. The generated client code remains browser-compatible.
Use kebab-case for CLI flag consistency with other flags like --dry-run, --keep-db, etc.
…y interface - Move programmatic API documentation to top (after installation) - Move CLI documentation to bottom as secondary interface - Remove legacy/backwards compatibility mentions and exports - Remove deprecation comments from code - Simplify barrel.ts by removing legacy boolean signature support - Clean up client-generator.ts comments - Update single-target.config.ts example comment
Remove GeneratorType, GenerateOrmOptions, GenerateOrmResult, and GenerateOrmTargetResult exports that were removed during legacy cleanup
…ed db object - Rename 'schema' to 'schemaFile' throughout codebase - Consolidate pgpmModulePath, pgpmWorkspacePath, pgpmModuleName into db.pgpm object - Move schemas, apiNames, keepDb into db object - Add db.config?: Partial<PgConfig> with fallback to environment variables - Update CLI to use --schema-file flag and new db structure - Update all generate commands, introspection sources, and config resolver - Update README with new config structure examples - Update tests for new config structure
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.
refactor(graphql-codegen): separate concerns by moving business logic to core
Summary
Refactors the
@constructive-io/graphql-codegenpackage to properly separate concerns by moving all business logic from thecli/directory into acore/directory structure. The CLI commands are now thin wrappers that handle user interaction while delegating to core modules.Structural changes:
cli/codegen/*→core/codegen/*(code generation)cli/introspect/*→core/introspect/*(schema introspection)cli/watch/*→core/watch/*(watch mode)cli/commands/shared.ts→core/pipeline/index.tsNew core modules:
core/config/- Config loading (loader.ts) and resolution (resolver.ts)core/output/- File writing utilities (writer.ts)core/database/- Database schema utilities (buildSchemaFromDatabase,buildSchemaSDLFromDatabase)core/introspect/source/database.ts- DatabaseSchemaSource for in-memory database introspectioncore/introspect/source/pgpm-module.ts- PgpmModuleSchemaSource for PGPM module introspectioncore/introspect/source/api-schemas.ts- Utilities for resolving schemas from API namescore/codegen/shared/- Shared types generator for unified outputKey pattern change: The watch orchestrator now accepts generate functions via dependency injection to avoid circular dependencies between CLI commands and core modules.
Updates since last revision
Phase 11: Database/PGPM config restructuring
Restructured the configuration to use a cleaner nested
dbobject pattern:schema→schemaFilethroughout codebasepgpmModulePath,pgpmWorkspacePath,pgpmModuleNameinto nesteddb.pgpmobjectschemas,apiNames,keepDbinto thedbobjectdb.config?: Partial<PgConfig>with fallback to environment variables viagetEnvOptions().pg--schema-fileflag and new db structureNew config structure:
Review & Testing Checklist for Human
dbconfig structure: Verify thatdb.pgpm.modulePath,db.schemas,db.apiNames, anddb.configall work correctly at runtime--schema-file(renamed from--schema),--pgpm-module-path,--schemas,--api-namesflagsdb.configis not provided, verify it correctly falls back toPGHOST,PGPORT,PGUSER,PGPASSWORD,PGDATABASEenvironment variablesresolveConfigTargets()correctly handles the new nesteddbstructure in defaults and targetsNotes
schemaconfig field renamed toschemaFiledatabase,pgpmModulePath,pgpmWorkspacePath,pgpmModuleName,schemas,apiNames,keepDbmoved into nesteddbobject--schemarenamed to--schema-filegenerateReactQueryandgenerateOrmare no longer exported - usegenerate({ reactQuery: true })orgenerate({ orm: true })ResolvedConfig,ResolvedWatchConfig,ResolvedQueryKeyConfigtypes are deleted - useGraphQLSDKConfigTarget,WatchConfig,QueryKeyConfigLink to Devin run: https://app.devin.ai/sessions/52a59f63cea74493bcb0d97f67422e8d
Requested by: Dan Lynch (@pyramation)