refactor: comprehensive cleanup and critical bug fixes#71
Open
productdevbook wants to merge 23 commits intomainfrom
Open
refactor: comprehensive cleanup and critical bug fixes#71productdevbook wants to merge 23 commits intomainfrom
productdevbook wants to merge 23 commits intomainfrom
Conversation
- fix(define): preserve __schema property on returned directive object instead of losing it during spread (was always undefined before) - fix(codegen): correct defu merge order so user config takes priority over defaults in client type generation - fix(codegen): enable typed-document-node plugin when documentMode is 'string' to generate TypedDocumentString class definition - fix(codegen): remove duplicate UseSubscriptionSessionReturn export - fix(routes/yoga): read endpoint from moduleConfig instead of hardcoded '/api/graphql', preserve response headers in early-return path - fix(routes/apollo): cache h3Handler to avoid recreation per request - fix(routes/health): use direct GraphQL execution instead of self-fetch which fails in serverless environments - fix(routes/debug): add runtime production guard to prevent accidental exposure - fix(rollup): remove dead return that disabled chunking, add advancedChunks and codeSplitting guards - fix(virtual.d.ts): remove orphan module declarations for server-scalars and client-schema that had no backing generators Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
graphql-yoga-ws.ts and apollo-server-ws.ts were byte-for-byte identical. Extract shared logic into _ws-handler.ts and reduce both files to thin wrappers that import wsHooks from the shared module. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Remove duplicate CoreSecurityConfig from core/server/types.ts - Remove duplicate directive types from nitro/types.ts, import from core/types/define.ts instead - Move StandardSchemaV1 to dedicated file (core/types/standard-schema.ts) - Replace GraphQLArgumentType 35-literal union with template literal type - Fix DEFAULT_PATHS_CONFIG null values to undefined for type safety Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Refactor 14-step monolithic setupNitroGraphQL into a sequential resolver chain (inspired by Nitro v3's config resolver pattern). - Each setup step is now an independent resolver function - Extract resolveSecurityConfig to dedicated security.ts module - Extract regenerateTypes to type-generation.ts (eliminates 3x copy-paste across setup, file-watcher, and close hooks) - Remove dead setupNuxtIntegration code - Remove dead switch branches in resolveBuildDirectories - Fix circular import between setup.ts and file-watcher.ts Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Scanner: - Use shared getImportId() instead of duplicated hash alias logic - Remove scanGraphqlCore (identical to scanSchemasCore) - Unify ignore patterns into DEFAULT_IGNORE_PATTERNS constant - Create ScanContext once and share across all 4 scanners - Fix parseSingleFile to log errors via consola.debug instead of silently swallowing them Codegen: - Pass schema string directly to avoid disk read round-trip - Parallelize external service type generation with Promise.all - Extract per-service logic into generateExternalServiceTypes Utilities: - Merge toPascalCase and capitalize into shared string.ts util - Export from core/utils barrel Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Regenerated SDK files now include TypedDocumentString class definition via typed-document-node plugin (documentMode: 'string' fix). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…re overhaul
- TypedDocumentString: replace raw string prepend with proper @graphql-codegen
plugin (src/core/codegen/plugins/typed-document-string.ts)
- Type system: make core types authoritative by extending actual codegen plugin
types (TypeScriptPluginConfig, TypeScriptResolversPluginConfig, etc.)
- Nitro types re-export from core with backward-compatible aliases
(CodegenServerConfig, CodegenClientConfig, GenericSdkConfig)
- Split src/nitro/codegen.ts (311 LOC) into codegen/{server,client,external}-types.ts
- Split src/nitro/virtual/generators.ts (291 LOC) into 10 focused modules
- Split src/nitro/types.ts (536 LOC) into types/{config,augmentation,define}.ts
- Eliminate 14 of 17 `as any` casts across src/ (remaining 3 in nuxt.ts @ts-nocheck)
- Strongly type CoreCodegenConfig and CoreExternalService in core/types/config.ts
- Simplify ScanDocumentsOptions to minimal interface (only uses documents field)
- Remove redundant CLI adapter externalServices mapping
- Rename mergedSdkConfig → resolvedSdkConfig with JSDoc
- Fix vitest coverage exclusion for new types directory structure
- Clean stale TODO and comments
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…e dead abstractions Architecture improvements identified through systematic review: - Split client.ts (665→242 LOC): extract subscription-extractor.ts (pure GraphQL) and vue-subscription-builder.ts (Vue composables) from framework-agnostic core - Unify SecurityConfig: delete duplicate in nitro/types, re-export from core (CoreSecurityConfig is now the single source of truth) - Remove dead ScanAdapter interface and implementation: scanning goes through core functions directly, the adapter layer was designed but never wired up - Type virtual module stubs: replace `any` with proper IResolvers, DebugInfoStub, and directive types for build-time type safety - Remove `(i: any)` casts from debug route (types now flow from stubs) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Create src/core/server/apollo.ts: Apollo Server factory following the same pattern as yoga.ts (shared CoreServerOptions, security defaults) - Slim Nitro Apollo route from 104→36 LOC: thin H3 wrapper calling core factory - Extract APOLLO_USER_FACING_ERROR_CODES constant, type formatError properly (GraphQLFormattedError instead of any) - Separate CLIGraphQLOptions from NitroGraphQLOptions: CLI-specific fields (rootDir, buildDir, ignore, watch, runtime) no longer pollute the module config API that users see in nitro.config.ts - Simplify CLIContext type to just CLIGraphQLOptions (was a complex intersection) - Type virtual module stubs properly (IResolvers, DebugInfoStub, null pubsub) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Replace DirectiveParser class + singleton with pure functions: parseDirectivesFromFile() replaces parser.parseDirectives() - Add OxcNode/OxcProperty interfaces for oxc-parser AST nodes, eliminating all `any` types from AST traversal (was 15+ occurrences) - Remove lazy-load pattern: use direct `parseSync` import from oxc-parser (already used this way in ast-scanner.ts) - Function is now synchronous (parseSync), removing unnecessary async overhead - Type ParsedDirective.defaultValue as union instead of `any` Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Type ast-scanner.ts parseExports: add OxcASTNode interface replacing
`{ body: any[] }` with typed AST node shape
- Type directive defaultValue: replace `any` with
`string | number | boolean | null` union (DirectiveDefaultValue)
- Type virtual.d.ts federation field: `any` → `Record<string, unknown>`
- Type apollo.ts context function: `any` → `TContext` with safe cast
- Type codegen pluginMap: `{ plugin: any }` → `{ plugin: unknown }`
- Remove dead WatchConfig.enabled field (was never read by file watcher)
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…tion - Fix Rolldown crash: check `'codeSplitting' in output` instead of truthiness — Rolldown throws when manualChunks is set alongside codeSplitting even when false - Fix extend package crash: warn and skip instead of throwing when package config is not found (e.g., workspace packages without nitro-graphql.config.ts during dev rescan) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Virtual module code generation reads nitro.scan* arrays during Rolldown dev build. The dev:start hook's rescan was resetting these arrays (scanLocalFiles), causing a window where extend schemas/resolvers were missing from virtual modules. Fix: skip rescan entirely when extend sources are configured, since extend packages don't change during dev. Also make scanLocalFiles collect all results before assigning to prevent partial state. Additionally: add debug logging to loadPackageConfig catch block (was silently swallowing all errors). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Config system cleanup based on full dependency analysis: - Remove CoreContext (dead — never called by any production code) - Remove CoreGraphQLOptions (over-engineered, 11 fields defined but only 4 populated) - Flatten CoreConfig: security/federation/codegen/externalServices as direct fields instead of nested in a never-fully-populated CoreGraphQLOptions bag - Remove FrameworkAdapter.createCoreContext (dead interface method) - Make ExternalGraphQLService extend ExternalServiceCodegenConfig (was fully redeclaring all fields without type relationship) - Make FederationConfig extend CoreFederationConfig (was parallel hierarchy) - Deduplicate LocalDirExtendSource: defined in core, re-exported from nitro - Remove mergeGraphQLOptions (unused utility) - Fix CLI buildDir default: `.nitro-graphql` → `.graphql` (matches DEFAULT_CLI_CONFIG) - Simplify both adapters to pass config fields directly Before: 3 parallel type hierarchies (ExternalService, Federation, Paths) After: single inheritance chain from core → nitro Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…tion
Two algorithmic improvements:
1. Virtual module eager snapshots:
- registerAllVirtualModules() now generates code eagerly and captures
it as a string, instead of storing a closure that reads nitro.scan*
lazily at Rolldown build time
- Eliminates the root cause of race conditions between concurrent
rescan (dev:start hook) and Rolldown virtual module resolution
- New refreshVirtualModules() explicitly re-snapshots after file
watcher rescans, ensuring hot reload works correctly
2. Path resolution simplification:
- Replace 7 separate compiled regexes with single `/{(\w+)}/g` pattern
- Single pass replacement via lookup table instead of 7 chained .replace()
- Extensible: adding a new placeholder requires only a lookup entry,
not a new regex constant
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Replace `import from '../index'` (barrel) with direct module imports to prevent circular dependency risk and ensure correct mock interception - Extract parseResolverFiles() and parseDirectiveFiles() helpers to DRY the repeated parse-loop pattern across scanPackageSource, scanLocalDirSource, and scanExplicitPaths - Use DEFAULT_IGNORE_PATTERNS constant instead of hardcoded array in scanLocalDirSource - Add emptyResult() factory to eliminate 6 repeated object literals - Parallelize schema/resolver/directive glob scans in scanLocalDirSource (was sequential, now Promise.all) - Update test mocks to match new direct import paths Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Extract repeated patterns in schema-loader.ts: - resolveServiceSources(): resolve headers + schemas from service config (was inline 3x) - buildLoaders(): auto-detect URL/file loaders from schema sources (was inline 4x) - loadWithLoaders(): single entry point for loadSchemaSync with auto loaders (was 4 variants) - resolveSchemaFilePath(): compute cache path (was inline 2x) - needsUpdate(): extract cache staleness check from downloadAndSaveSchema (was deeply nested in a multi-branch conditional) 227 → 181 LOC, zero behavior change, same test coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…an arrays BREAKING STRUCTURAL CHANGE: The scan pipeline now produces frozen state snapshots instead of mutating 6 separate arrays on the Nitro instance. New architecture: scanLocalFiles() → createScanState() → frozen GraphQLScanState resolveExtendConfig() → mergeScanState() → new frozen snapshot performGraphQLScan() → nitro.graphql.state = finalState (atomic write) All consumers (virtual modules, codegen, logging) now read from the single `nitro.graphql.state` snapshot. State is never mutated — updates produce new frozen objects via Object.freeze(). Benefits: - Race conditions impossible: state is immutable once assigned - Atomic updates: no partial state visible to concurrent readers - Single source of truth: nitro.graphql.state replaces 6 mutable arrays - Extend merge is pure: mergeScanState(base, extend) → new state Backward compatibility maintained: - Legacy fields (nitro.scanSchemas, nitro.graphql.extendConfigs, etc.) are synced from state after each scan for existing tests/consumers - All fields marked @deprecated with migration guidance - All 1017 tests pass without changes to test assertions New files: - src/nitro/state.ts — emptyScanState, createScanState, mergeScanState - src/nitro/types/augmentation.ts — GraphQLScanState interface Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…works - Extract resolveSecurityDefaults() to core/server/types.ts — single source of truth for security config defaults, used by both Yoga and Apollo factories (was duplicated as inline object literal in each) - Move BASE_SCHEMA_DEF (SchemaDefinition wrapper) to core/schema/builder.ts alongside the BASE_SCHEMA string constant it wraps - Add BASE_SCHEMA_DEF to Apollo route (was missing — extend type Query syntax requires base Query/Mutation types to exist) - Both route handlers now follow identical pattern: schemas: [BASE_SCHEMA_DEF, ...schemas] Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…ding Based on fresh-eyes readability audit: Naming fixes: - Rename resolvedSdkConfig → sdkFileConfig in nitro/codegen/ to avoid collision with core/codegen/client.ts's resolvedSdkConfig (different types) - Mark CodegenServerConfig, CodegenClientConfig, GenericSdkConfig as @deprecated in public exports (prefer ServerCodegenConfig etc.) - Mark FILE_CONTEXT_TS constant as @deprecated with JSDoc Documentation: - Add module-level comment to paths.ts explaining its dual responsibility (path placeholders + generation control) - Document nitro.graphql.dir vs *Dir fields (relative vs absolute paths) - Explain defu merge order and .reverse() trick in graphql-config virtual module - Add BASE_SCHEMA_DEF purpose comment at both route handler call sites - Replace '<directives>' magic string with descriptive '<inline-directives>' Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Replace GenImport/IESMImport with ScannedResolver as single source of truth - Rename config.ts → create-config.ts, defaults.ts, file-header.ts, file-scanner.ts, runtime-generator.ts - Split watcher/index.ts → create-watcher.ts + barrel - Split pubsub/index.ts → memory-pubsub.ts + barrel - Fix BASE_SCHEMA import (was importing from wrong module) - Remove dead GenImport type alias and stale comments Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
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
1 participant
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
Test plan
pnpm vitest run— 44 files, 1017 tests passpnpm build— 206 files, cleanpnpm playground:nitrobuild — schemas: 4, resolvers: 5, server built successfullypnpm playground:nuxtpnpm playground:federation🤖 Generated with Claude Code