refactor: comprehensive cleanup and critical bug fixes#71
Merged
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
- Delete 30+ unused constants (PLACEHOLDER_*, RESOLVER_TYPE_*, HTTP_STATUS_*, etc.) - Remove dead async I/O functions and pathe re-exports from runtime.ts - Remove dead defaultLogger, createSilentLogger from logger.ts - Remove dead DEFAULT_IGNORE_PATTERNS, DEFAULT_PATHS_CONFIG, DEFAULT_WATCHER_* from defaults.ts - Remove dead readFileSafe from file-io.ts, ScalarType/SchemaLoadOptions from codegen.ts - De-duplicate Flatten<T> (nitro/types/define.ts now re-exports from core) - Unexport module-private escapeHtml, DirectiveDefaultValue - Delete dead barrels: cli/commands/index.ts, codegen/plugins/index.ts 252 → 248 files, 423 → 409 kB Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete apollo playground (federation already tests both frameworks) - Delete webhook playground (empty, no source files) - Replace vite User CRUD with minimal hello schema (was duplicating nitro) Remaining playgrounds (6): nitro — kitchen sink (directives, extend, client docs) nuxt — Nuxt integration + Rolldown federation — Apollo Federation, dual framework configs subscriptions — WebSocket/SSE, PubSub, Vue frontend cli — CLI commands, extend source package vite — Vite plugin integration, DevTools, custom serverDir Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Hoist regex literals to module scope (e18e/prefer-static-regex) - Remove stale @param JSDoc tag (jsdoc/check-param-names) - Remove unused vars in tests (unused-imports/no-unused-vars) - Fix trailing blank lines and export ordering (auto-fixed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
- Move typed-document-string.ts out of plugins/ subdirectory to codegen root - Delete empty plugins/ directory - Extract server type helpers into server-type-helpers.ts (was inline string) - Create serverImportsPlugin() — named plugin instead of anonymous inline - De-duplicate GENERATED_FILE_HEADER — derive from single HEADER_LINES source - Remove re-exports from client.ts (barrel's responsibility) - Use GENERATED_FILE_HEADER in generateGenericSdkContent() - Remove deprecated generateTypes() wrapper and its tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Custom @graphql-codegen plugin wrappers (pluginContent, typedDocumentStringPlugin, serverImportsPlugin) were unnecessary abstractions — they just prepended strings. Replaced with direct string concatenation on codegen output. - pluginContent → GENERATED_FILE_HEADER constant (string prepend) - typedDocumentStringPlugin → TYPED_DOCUMENT_STRING_CLASS constant (string prepend) - serverImportsPlugin → generateServerPrepend() returns string (string prepend) - codegen() calls now use only official @graphql-codegen plugins Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…g workaround documentMode: 'string' required maintaining a custom TypedDocumentString class because import-types-preset uses `import type` which can't bring in runtime values. Instead of maintaining this workaround, remove the default and let @graphql-codegen use its own default behavior. - Remove documentMode: 'string' from DEFAULT_CLIENT_CODEGEN_CONFIG - Delete typed-document-string.ts (no longer needed) - Remove TypedDocumentString prepend logic from client.ts Users who need string mode can set documentMode: 'string' explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Generated output updated after removing documentMode: 'string' default. SDK now uses gql tagged template literals instead of TypedDocumentString. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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