fix: vite open api server integrate loaders #44
Merged
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
added 8 commits
January 16, 2026 12:17
## What Changed - Replaced function-based handler/seed types with code-based types - Handlers now export objects mapping operationId → JS code string - Seeds now export objects mapping schemaName → JS code string - Both support static strings or dynamic functions that generate code ## New Types Handler Types: - HandlerCodeContext: Context for dynamic code generators - HandlerCodeGeneratorFn: Function that returns code string - HandlerValue: string | HandlerCodeGeneratorFn - HandlerExports: Record<operationId, HandlerValue> - HandlerLoadResult, ResolvedHandlers Seed Types: - SeedCodeContext: Context for dynamic code generators - SeedCodeGeneratorFn: Function that returns code string - SeedValue: string | SeedCodeGeneratorFn - SeedExports: Record<schemaName, SeedValue> - SeedLoadResult, ResolvedSeeds ## Why The Scalar Mock Server expects x-handler/x-seed as JavaScript code strings, not runtime functions. This aligns with PRD FR-004/FR-005. ## Temporary Changes (TODO) - Loaders updated with minimal changes, full rewrite pending - Document enhancer updated with minimal changes, full rewrite pending Closes: vite-open-api-server-thy.1
… → code mappings
## What Changed
- Handler files now export objects: { operationId: string | function }
- Static handlers are code strings injected directly as x-handler
- Dynamic handlers are functions that generate code based on context
- Loader validates object exports (not functions or arrays)
- Returns HandlerLoadResult with handlers, loadedFiles, warnings, errors
## Implementation
- loadHandlerFile(): Process each key-value pair from exports object
- isValidExportsObject(): Validate default export is plain object
- isValidHandlerValue(): Validate each value is string or function
- checkOperationExists(): Cross-reference with registry
- getHandlerType(): Return human-readable type for logging
## Updated Tests
- All fixtures converted to new object export format
- Tests verify static (string) and dynamic (function) handlers
- Tests verify HandlerLoadResult structure
Closes: vite-open-api-server-thy.2
BREAKING CHANGE: Seed files must now export an object mapping schemaName
to seed values instead of a single function.
## Changes
- Rewrite seed-loader.ts to load object exports { schemaName: string | fn }
- Return SeedLoadResult with seeds Map, loadedFiles, warnings, errors
- Add validation for object exports (reject functions, arrays)
- Cross-reference schemaNames with registry.schemas for validation
- Keep singular/plural matching utilities for schema name lookups
## Fixtures Updated
- pets.seed.mjs: Export object with Pet (static) and Category (dynamic)
- Order.seed.mjs: Export object with Order (dynamic)
- invalid-not-function.seed.mjs: Now exports function (invalid)
- invalid-array-export.seed.mjs: New fixture for array export (invalid)
- subdirectory/pets.seed.mjs: Export object for duplicate testing
## Tests
- 59 tests covering: object exports, static/dynamic seeds, duplicates,
registry cross-reference, error handling, utility functions
Closes: vite-open-api-server-thy.3
BREAKING CHANGE: enhanceDocument() is now async and returns Promise<EnhanceDocumentResult>. ## Changes - enhanceDocument() now resolves handler/seed values to code strings before injection - Static values (strings) are used directly - Dynamic values (functions) are called with context to generate code strings - Handler context includes: operationId, path, method, operation, document, schemas - Seed context includes: schemaName, schema, document, schemas - Both sync and async generator functions are supported - Error handling for failed resolution with continued processing ## Tests - 49 tests covering: static/dynamic handlers, static/dynamic seeds, context passing, async functions, error handling, mixed values Closes: vite-open-api-server-thy.4
- Import loadHandlers and loadSeeds from loaders module - Import enhanceDocument from enhancer module - Load custom handlers if HANDLERS_DIR is configured - Load custom seeds if SEEDS_DIR is configured - Call enhanceDocument to inject x-handler and x-seed extensions - Pass enhanced document (with extensions) to Scalar createMockServer - Log summary of loaded handlers/seeds and enhancement stats Closes: vite-open-api-server-thy.5
Replace function-based handlers and seeds with code-based format that follows the PRD specification and Scalar Mock Server x-handler/x-seed conventions. ## Handlers (new format) - pets.handler.ts: All pet operations (findPetsByStatus, getPetById, addPet, etc.) - store.handler.ts: Store operations (getInventory, placeOrder, getOrderById, etc.) - users.handler.ts: User operations (createUser, loginUser, getUserByName, etc.) Each handler exports an object mapping operationId to JavaScript code strings that Scalar will execute with access to store, faker, req, and res. ## Seeds (updated format) - pets.seed.ts: Pet, Category, Tag schemas with realistic data - orders.seed.ts: Order schema with workflow states - users.seed.ts: User schema with test user (user1/password123) Each seed exports an object mapping schemaName to JavaScript code strings that use seed.count() to populate the in-memory store. Deleted (old function-based format): - add-pet.handler.ts - delete-pet.handler.ts - get-pet-by-id.handler.ts - update-pet.handler.ts Closes: vite-open-api-server-thy.6
- Change from named import { glob } to default import fg from 'fast-glob'
- Use fg.glob() instead of glob() to call the function
- Fixes 'Named export glob not found' error when running in Node ESM mode
Also convert playground handler/seed files from .ts to .mjs:
- Node.js cannot import TypeScript files directly without a loader
- Changed to pure JavaScript .mjs files for runtime compatibility
- Removed TypeScript imports and type annotations
Tested end-to-end:
- Handler loading: 19 handlers from 3 files ✓
- Seed loading: 5 seeds from 3 files ✓
- x-handler/x-seed injection into OpenAPI spec ✓
- API responses with seeded data (faker-generated pets) ✓
Closes: vite-open-api-server-thy.7
miguelramos
added
🦠 TYPE: Bug
📝 WalkthroughWalkthroughConvert handler/seed modules from runtime functions to code-generation values (static strings or generator functions), make loaders return structured result objects with metadata and validation, make document enhancement async to resolve values before injection, and update types, tests, fixtures, runner integration, and loader utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as OpenApiServerRunner
participant HLoader as HandlerLoader
participant SLoader as SeedLoader
participant Enhancer as DocumentEnhancer
participant Mock as MockServer
Runner->>HLoader: loadHandlers(dir, registry, logger)
HLoader->>HLoader: discover files, import, validate, build HandlerLoadResult
HLoader-->>Runner: HandlerLoadResult { handlers, loadedFiles, warnings, errors }
Runner->>SLoader: loadSeeds(dir, registry, logger)
SLoader->>SLoader: discover files, import, validate, build SeedLoadResult
SLoader-->>Runner: SeedLoadResult { seeds, loadedFiles, warnings, errors }
Runner->>Enhancer: enhanceDocument(spec, handlers, seeds, logger)
Enhancer->>Enhancer: resolveHandlerValue(operationId, value, spec, ctx, schemas)
Enhancer->>Enhancer: resolveSeedValue(schemaName, value, spec, schema, schemas)
Enhancer-->>Runner: EnhanceDocumentResult { document, handlerCount, seedCount, overrideCount }
Runner->>Mock: createMockServer(documentForMockServer)
Mock-->>Runner: ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
- Fix import organization in handler-loader.ts (node imports before external) - Fix import organization in seed-loader.ts (node imports before external) - Replace non-null assertions with optional chaining in document-enhancer.test.ts Refs: vite-open-api-server-thy
Member
Author
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. Summary of Changes:
The implementation correctly follows PRD specification FR-004 and FR-005. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vite-plugin-open-api-server/src/runner/openapi-server-runner.mts (1)
176-293: Refactormainto reduce cognitive complexity and fix CI lint failure.
CI reports complexity 24 > 15 at Line 176. Extract handler/seed loading and document enhancement into helpers to pass lint and keep the runner maintainable.♻️ Suggested refactor to reduce complexity
+async function loadCustomHandlers( + handlersDir: string | undefined, + registry: Parameters<typeof loadHandlers>[1], + logger: Parameters<typeof loadHandlers>[2], + verbose: boolean, +): Promise<Map<string, HandlerValue>> { + if (!handlersDir) return new Map(); + if (verbose) console.log(`[mock-server] Loading handlers from: ${handlersDir}`); + const result = await loadHandlers(handlersDir, registry, logger); + if (result.errors.length > 0) { + console.warn(`[mock-server] Handler loading had ${result.errors.length} error(s)`); + } + return result.handlers; +} + +async function loadCustomSeeds( + seedsDir: string | undefined, + registry: Parameters<typeof loadSeeds>[1], + logger: Parameters<typeof loadSeeds>[2], + verbose: boolean, +): Promise<Map<string, SeedValue>> { + if (!seedsDir) return new Map(); + if (verbose) console.log(`[mock-server] Loading seeds from: ${seedsDir}`); + const result = await loadSeeds(seedsDir, registry, logger); + if (result.errors.length > 0) { + console.warn(`[mock-server] Seed loading had ${result.errors.length} error(s)`); + } + return result.seeds; +} + +async function enhanceDocumentIfNeeded( + spec: OpenAPIV3_1.Document, + handlers: Map<string, HandlerValue>, + seeds: Map<string, SeedValue>, + logger: Parameters<typeof enhanceDocument>[3], + verbose: boolean, +): Promise<OpenAPIV3_1.Document> { + if (handlers.size === 0 && seeds.size === 0) return spec; + if (verbose) { + console.log( + `[mock-server] Enhancing document with ${handlers.size} handler(s) and ${seeds.size} seed(s)`, + ); + } + const result = await enhanceDocument(spec, handlers, seeds, logger); + console.log( + `[mock-server] Document enhanced: ${result.handlerCount} handler(s), ${result.seedCount} seed(s)` + + (result.overrideCount > 0 ? `, ${result.overrideCount} override(s)` : ''), + ); + return result.document; +} @@ - let handlers = new Map<string, HandlerValue>(); - if (config.handlersDir) { - if (config.verbose) { - console.log(`[mock-server] Loading handlers from: ${config.handlersDir}`); - } - const handlerResult = await loadHandlers( - config.handlersDir, - registry, - consoleLogger as Parameters<typeof loadHandlers>[2], - ); - handlers = handlerResult.handlers; - - if (handlerResult.errors.length > 0) { - console.warn(`[mock-server] Handler loading had ${handlerResult.errors.length} error(s)`); - } - } - - let seeds = new Map<string, SeedValue>(); - if (config.seedsDir) { - if (config.verbose) { - console.log(`[mock-server] Loading seeds from: ${config.seedsDir}`); - } - const seedResult = await loadSeeds( - config.seedsDir, - registry, - consoleLogger as Parameters<typeof loadSeeds>[2], - ); - seeds = seedResult.seeds; - - if (seedResult.errors.length > 0) { - console.warn(`[mock-server] Seed loading had ${seedResult.errors.length} error(s)`); - } - } - - let documentForMockServer = specAsOpenAPI; - if (handlers.size > 0 || seeds.size > 0) { - if (config.verbose) { - console.log( - `[mock-server] Enhancing document with ${handlers.size} handler(s) and ${seeds.size} seed(s)`, - ); - } - const enhanceResult = await enhanceDocument( - specAsOpenAPI, - handlers, - seeds, - consoleLogger as Parameters<typeof enhanceDocument>[3], - ); - documentForMockServer = enhanceResult.document; - - console.log( - `[mock-server] Document enhanced: ${enhanceResult.handlerCount} handler(s), ${enhanceResult.seedCount} seed(s)` + - (enhanceResult.overrideCount > 0 ? `, ${enhanceResult.overrideCount} override(s)` : ''), - ); - } + const handlers = await loadCustomHandlers( + config.handlersDir, + registry, + consoleLogger as Parameters<typeof loadHandlers>[2], + config.verbose, + ); + const seeds = await loadCustomSeeds( + config.seedsDir, + registry, + consoleLogger as Parameters<typeof loadSeeds>[2], + config.verbose, + ); + const documentForMockServer = await enhanceDocumentIfNeeded( + specAsOpenAPI, + handlers, + seeds, + consoleLogger as Parameters<typeof enhanceDocument>[3], + config.verbose, + );
🤖 Fix all issues with AI agents
In @.changesets/fix-vite-open-api-server-thy-integrate-loaders.json:
- Around line 1-13: This changeset indicates a breaking change but still sets
"bump" to "minor" and leaves "changes" empty; update the "bump" field to "major"
and populate the "changes" array with concise release-note entries describing
the breaking API removals (remove types HandlerContext, SeedContext,
HandlerResponse, SeedData) and the addition of the new code-generation types for
`@websublime/vite-plugin-open-api-server`; include a short migration note
referencing the removed types and any recommended replacement types or steps so
consumers can migrate.
In
`@packages/vite-plugin-open-api-server/src/enhancer/__tests__/document-enhancer.test.ts`:
- Around line 559-562: The test uses a non-null assertion on operationInfo (from
findOperationById) which violates noNonNullAssertion; change it to an explicit
guard: assert operationInfo is defined (e.g.,
expect(operationInfo).not.toBeNull()/toBeDefined() or if (!operationInfo) throw
new Error(...)) before calling getExtension(operationInfo.operation,
'x-handler'), then perform the expect on the returned value; target the
findOperationById call and the operationInfo variable in
document-enhancer.test.ts to implement this guard.
- Around line 349-392: The tests use non-null assertions (e.g., operationInfo!
and getExtension(..., 'x-handler')) which fails lint; update the dynamic handler
tests to guard against nulls by asserting or early-failing when
findOperationById returns null before calling getExtension: call const
operationInfo = findOperationById(...); expect(operationInfo).not.toBeNull(); if
(!operationInfo) throw new Error('operation not found'); then safely read
getExtension(operationInfo.operation, 'x-handler') and access properties on the
mocked context; apply the same null-guard pattern for other uses of
operationInfo and ensure context extraction (contextSpy.mock.calls[0][0]) is
only done after verifying the spy was called.
- Around line 815-821: Replace non-null assertions in the mixed handler tests by
adding explicit guards before using the operations: check that
findOperationById(result.document, 'listPets') and
findOperationById(result.document, 'getPetById') return a value (e.g.,
assert/expect toBeDefined or if (!...) throw/return) and then call getExtension
on those guarded objects; likewise ensure dynamicCode is only used after
confirming getExtension returned a string (guard against undefined) before
calling toContain. Update references to listPetsOp, getPetByIdOp, getExtension,
and dynamicCode accordingly so no `!` non-null assertions remain.
- Around line 306-329: The tests use non-null assertions (listPetsOp! and
createPetOp!) which fail lint; after asserting the operation exists
(expect(listPetsOp).not.toBeNull()), add an explicit runtime guard that narrows
the type before calling getExtension (e.g., if (!listPetsOp) throw new
Error(...) or return), then call getExtension(listPetsOp.operation,
'x-handler'); do the same for createPetOp; reference findOperationById to obtain
the ops and getExtension to read the extension so the lint rule
noNonNullAssertion is satisfied.
In `@packages/vite-plugin-open-api-server/src/enhancer/document-enhancer.ts`:
- Around line 169-251: The resolvers resolveHandlerValue and resolveSeedValue
both rebuild the same schemas map on every call causing redundant O(n*m) work;
extract the logic that builds the schemas Record<string, SchemaObject> into a
single cached function or a precomputed variable created once per
enhancement/inject phase (e.g., computeSchemas(spec) or precomputeSchemas in the
enhancer initialization) and have both resolveHandlerValue and resolveSeedValue
accept or reference that cached schemas map instead of recomputing it each call;
ensure the cached map is built from spec.components?.schemas and filters out
ReferenceObjects the same way as the existing loops.
In
`@packages/vite-plugin-open-api-server/src/loaders/__tests__/fixtures/invalid-not-function.handler.mjs`:
- Around line 1-9: The fixture file named invalid-not-function.handler.mjs is
misleading because the exported value is an array (invalid array export), so
rename the fixture to invalid-array-export.handler.mjs and update any
tests/imports that reference invalid-not-function.handler.mjs (and any test
descriptions or snapshot names) to use the new filename; ensure the file content
remains the same (export default [ ... ]) so the loader test continues to
validate an invalid array-format handler export.
In
`@packages/vite-plugin-open-api-server/src/loaders/__tests__/handler-loader.test.ts`:
- Around line 134-145: The test fails because loadHandlers currently lets
fg.glob throw ENOENT and the outer catch pushes a fatal error into errors and
calls logger.error; change loadHandlers to proactively check the handlers
directory existence (e.g., fs.existsSync or fs.stat) before calling fg.glob and,
if the directory does not exist, push a warning into warnings, call logger.warn
with a message containing "No handler files found", and return the empty
handlers result; keep the existing outer try-catch for other runtime errors so
other exceptions still populate errors and call logger.error.
In
`@packages/vite-plugin-open-api-server/src/loaders/__tests__/seed-fixtures/invalid-not-function.seed.mjs`:
- Around line 1-15: Rename the test fixture file to accurately reflect that it
exports a function: change the filename from invalid-not-function.seed.mjs to
something like invalid-function-export.seed.mjs (or
invalid-is-function.seed.mjs) so the file name matches the default async
function export invalidSeed and clearly communicates the invalid case being
tested; ensure any test imports or references to the old filename are updated to
the new filename.
In `@packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts`:
- Around line 25-31: Imports in handler-loader.ts are misordered causing Biome's
organize-imports failure; run Biome's organize-imports or manually reorder the
import statements so they follow the project's import ordering rules (external
packages first, then node builtins, then URL helpers, then types) — specifically
adjust the block importing fg, node:path, node:url (pathToFileURL), vite Logger,
and the local type imports HandlerExports/HandlerLoadResult/HandlerValue and
OpenApiEndpointRegistry so they are grouped and sorted correctly; after
reordering, re-run the linter to confirm the error is resolved.
In `@packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts`:
- Around line 25-31: CI flagged import ordering in the top of seed-loader.ts;
run the project's organizer/formatter (Biome) or manually reorder the imports so
they satisfy Biome's organizeImports rules: group and alphabetize node builtins
(node:path, node:url) together before external modules (fast-glob), keep value
imports separate from type-only imports (Logger, OpenApiEndpointRegistry,
SeedExports, SeedLoadResult, SeedValue) and alphabetize within each group, then
re-run the formatter to clear the warning.
packages/vite-plugin-open-api-server/src/enhancer/__tests__/document-enhancer.test.ts
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/enhancer/__tests__/document-enhancer.test.ts
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/enhancer/__tests__/document-enhancer.test.ts
Outdated
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/enhancer/__tests__/document-enhancer.test.ts
Show resolved
Hide resolved
Comment on lines
1
to
9
| /** | ||
| * Invalid handler fixture for testing. | ||
| * Default export is not a function (it's an object). | ||
| * Default export is an array, which is not a valid handler exports format. | ||
| * Handler files must export a plain object mapping operationId → handler value. | ||
| */ | ||
| export default { | ||
| handler: async (_context) => { | ||
| return { | ||
| status: 200, | ||
| body: { error: 'This should not work' }, | ||
| }; | ||
| }, | ||
| name: 'invalidHandler', | ||
| }; | ||
| export default [ | ||
| { operationId: 'getPet', code: 'return store.get("Pet", req.params.petId);' }, | ||
| { operationId: 'addPet', code: 'return store.create("Pet", req.body);' }, | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider renaming fixture for clarity.
The fixture now tests that arrays are invalid handler exports (the loader expects plain objects mapping operationId → handler value). However, the filename invalid-not-function.handler.mjs no longer accurately describes the test case—it's now testing an invalid array format, not a non-function export.
Consider renaming to something like invalid-array-export.handler.mjs to better reflect the validation being tested.
🤖 Prompt for AI Agents
In
`@packages/vite-plugin-open-api-server/src/loaders/__tests__/fixtures/invalid-not-function.handler.mjs`
around lines 1 - 9, The fixture file named invalid-not-function.handler.mjs is
misleading because the exported value is an array (invalid array export), so
rename the fixture to invalid-array-export.handler.mjs and update any
tests/imports that reference invalid-not-function.handler.mjs (and any test
descriptions or snapshot names) to use the new filename; ensure the file content
remains the same (export default [ ... ]) so the loader test continues to
validate an invalid array-format handler export.
packages/vite-plugin-open-api-server/src/loaders/__tests__/handler-loader.test.ts
Show resolved
Hide resolved
Comment on lines
1
to
15
| /** | ||
| * Invalid Seed Fixture - Not a Function | ||
| * Invalid Seed Fixture - Function Export (Not Object) | ||
| * | ||
| * This seed file exports a non-function default export. | ||
| * This seed file exports a function as default export, which is invalid. | ||
| * Seed files must export a plain object mapping schemaName to seed values. | ||
| * Used to test that the seed loader properly validates exports. | ||
| */ | ||
|
|
||
| // Invalid: default export is not a function | ||
| export default { | ||
| id: 1, | ||
| name: 'Invalid Seed', | ||
| data: 'This is not a function', | ||
| }; | ||
| // Invalid: default export is a function, not an object | ||
| export default async function invalidSeed(_context) { | ||
| return [ | ||
| { id: 1, name: 'Invalid Seed 1' }, | ||
| { id: 2, name: 'Invalid Seed 2' }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Filename contradicts file content.
The filename invalid-not-function.seed.mjs suggests the export is "not a function," but the file actually exports an async function. This naming inconsistency could confuse developers maintaining the tests.
Consider renaming to invalid-function-export.seed.mjs or invalid-is-function.seed.mjs to accurately describe the invalid case being tested (exporting a function when an object is expected).
🤖 Prompt for AI Agents
In
`@packages/vite-plugin-open-api-server/src/loaders/__tests__/seed-fixtures/invalid-not-function.seed.mjs`
around lines 1 - 15, Rename the test fixture file to accurately reflect that it
exports a function: change the filename from invalid-not-function.seed.mjs to
something like invalid-function-export.seed.mjs (or
invalid-is-function.seed.mjs) so the file name matches the default async
function export invalidSeed and clearly communicates the invalid case being
tested; ensure any test imports or references to the old filename are updated to
the new filename.
Comment on lines
25
to
31
| import fg from 'fast-glob'; | ||
| import path from 'node:path'; | ||
| import { pathToFileURL } from 'node:url'; | ||
| import { glob } from 'fast-glob'; | ||
| import type { Logger } from 'vite'; | ||
|
|
||
| import type { HandlerCodeGenerator } from '../types/handlers.js'; | ||
| import type { HandlerExports, HandlerLoadResult, HandlerValue } from '../types/handlers.js'; | ||
| import type { OpenApiEndpointRegistry } from '../types/registry.js'; |
There was a problem hiding this comment.
Resolve Biome import-order error (CI).
This file is currently failing organizeImports. Please run Biome’s organize-imports to fix the error.
🧰 Tools
🪛 GitHub Actions: CI
[error] 25-25: assist/source/organizeImports: The imports and exports are not sorted. Safe fix: Organize Imports (Biome).
🤖 Prompt for AI Agents
In `@packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts` around
lines 25 - 31, Imports in handler-loader.ts are misordered causing Biome's
organize-imports failure; run Biome's organize-imports or manually reorder the
import statements so they follow the project's import ordering rules (external
packages first, then node builtins, then URL helpers, then types) — specifically
adjust the block importing fg, node:path, node:url (pathToFileURL), vite Logger,
and the local type imports HandlerExports/HandlerLoadResult/HandlerValue and
OpenApiEndpointRegistry so they are grouped and sorted correctly; after
reordering, re-run the linter to confirm the error is resolved.
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts`:
- Around line 187-250: Duplicate validation and logging helpers
(isValidExportsObject, isValidHandlerValue/isValidSeedValue,
getHandlerType/getSeedType, logLoadSummary) should be extracted to a shared
module (e.g., loader-utils.ts); create exported helpers (isValidExportsObject,
isValidValue, getValueType, logLoadSummary) in loader-utils.ts, replace the
local implementations in handler-loader.ts and seed-loader.ts with imports from
loader-utils, update usages to use the new generic names (e.g., replace
isValidHandlerValue/isValidSeedValue with isValidValue and
getHandlerType/getSeedType with getValueType) and keep existing function
signatures/types (HandlerValue/seed types) so callers like checkOperationExists
and logLoadSummary continue to work unchanged.
In `@packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts`:
- Around line 188-197: The isValidExportsObject function contains a redundant
check (typeof value !== 'function') because you already verify typeof value ===
'object'; remove the unnecessary function-type guard to simplify the predicate
in isValidExportsObject while keeping the other checks (non-null, not an array,
and Object.getPrototypeOf(value) === Object.prototype) intact.
♻️ Duplicate comments (2)
.changesets/fix-vite-open-api-server-thy-integrate-loaders.json (1)
3-3: Breaking change label conflicts with a minor bump.The PR is labeled as a breaking change, but this changeset still declares a
"minor"bump. Align the bump to"major"or confirm this isn’t actually breaking.packages/vite-plugin-open-api-server/src/enhancer/__tests__/document-enhancer.test.ts (1)
306-330: Add null assertions before optional chaining for better test failure messages.Lines 323-329 use optional chaining without prior null assertions. If
findOperationByIdreturnsnull, the test would pass withundefined === 'return store.list("Pet");'being false, but the assertion failure message would be confusing. Adding explicit null checks improves test diagnostics.🧪 Suggested improvement for clearer test failures
const listPetsOp = findOperationById(result.document, 'listPets'); + expect(listPetsOp).not.toBeNull(); expect(getExtension(listPetsOp?.operation, 'x-handler')).toBe('return store.list("Pet");'); const createPetOp = findOperationById(result.document, 'createPet'); + expect(createPetOp).not.toBeNull(); expect(getExtension(createPetOp?.operation, 'x-handler')).toBe( 'return store.create("Pet", req.body);', );
packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts
Outdated
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
added 2 commits
January 16, 2026 14:01
- Extract shared loader utilities to loader-utils.ts (isValidExportsObject, isValidValue, getValueType, logLoadSummary) - Cache schemas extraction in document-enhancer to avoid redundant O(n*m) work - Add null guards in tests instead of non-null assertions (noNonNullAssertion lint rule) - Check directory existence before scanning in loaders (prevents ENOENT errors) - Rename invalid-not-function.handler.mjs to invalid-array-export.handler.mjs - Rename invalid-not-function.seed.mjs to invalid-function-export.seed.mjs - Fix import ordering per Biome rules
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts`:
- Around line 207-217: The current checkOperationExists function scans
registry.endpoints.values() for each handler causing O(n*m) cost; change
loadHandlers to prebuild a Set of operationIds from OpenApiEndpointRegistry
(e.g., from registry.endpoints.values()) and replace checkOperationExists calls
with lookup against that operationIdSet, ensuring you filter out falsy
operationIds when constructing the set and keep or remove the old
checkOperationExists function accordingly.
In `@packages/vite-plugin-open-api-server/src/loaders/loader-utils.ts`:
- Around line 144-154: The type description in formatInvalidExportError
currently maps null to "object" because typeof null === 'object'; update
formatInvalidExportError to detect null (actualValue === null) and set a clearer
typeDesc such as 'null' (or 'null (object)') before the existing Array.isArray
and typeof function checks so error messages for null default exports read "Got:
null" instead of "Got: object".
In `@packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts`:
- Around line 205-227: checkSchemaExists currently duplicates candidate
generation logic that findMatchingSchema already implements, risking drift;
replace the body of checkSchemaExists(schemaName, registry) to call
findMatchingSchema(schemaName, registry) and return a boolean (true if
findMatchingSchema returns a match, false otherwise), and remove the local
candidate-generation code so all schema-matching behavior is centralized via
findMatchingSchema while still querying registry.schemas for existence through
that helper.
- Around line 151-163: The presence check for the default export uses a truthy
test (!module.default) which misidentifies valid falsy defaults; change the
check to test property presence (e.g., using 'default' in module) so
missing-default and invalid-type errors are distinct, then let the existing
isValidExportsObject(exports) validate that the default export is an object;
update the check around module.default and the subsequent const exports =
module.default accordingly.
- Around line 88-100: The code in seed-loader currently uses
fs.existsSync(absoluteDir) to decide whether to glob for seed files, but that
only checks existence and not that absoluteDir is a directory; replace that
check with a directory test (use fs.statSync(absoluteDir).isDirectory() or
fs.lstatSync(...).isDirectory()) so that if seedsDir points to a file you treat
it like "no seed files" rather than letting fg.glob throw ENOTDIR; update the
branch that logs `[seed-loader] No seed files found in ${seedsDir}` and pushes
warnings to return early when the path exists but is not a directory, keeping
the same warnings/errors variables and return shape used by the surrounding
function.
♻️ Duplicate comments (2)
packages/vite-plugin-open-api-server/src/loaders/__tests__/handler-loader.test.ts (1)
134-146: LGTM!The test correctly expects warnings (not errors) for non-existent directories. This aligns with the implementation fix that added
fs.existsSynccheck before callingfg.glob, resolving the previous review concern about ENOENT errors.packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts (1)
25-38: LGTM!Import organization follows proper grouping (Node.js builtins, external packages, local types, local utilities). The shared utilities from
loader-utils.jsare correctly imported, addressing the previous review feedback about code duplication.
packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts
Outdated
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
added 2 commits
January 16, 2026 14:13
- handler-loader: Pre-build operationId Set for O(1) lookups instead of O(n*m) scanning in checkOperationExists; remove the function and use Set.has() directly - handler-loader: Add directory check (not just existence) before globbing - handler-loader: Use 'default' in module check instead of truthy check for exports - seed-loader: Refactor checkSchemaExists to delegate to findMatchingSchema, centralizing schema-matching logic and avoiding duplication - seed-loader: Add directory check (not just existence) before globbing to prevent ENOTDIR errors when seedsDir points to a file - seed-loader: Use 'default' in module check instead of truthy check to distinguish missing-default from falsy-default errors - loader-utils: Handle null explicitly in formatInvalidExportError for clearer error messages (null now shows 'null' instead of 'object')
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/vite-plugin-open-api-server/src/loaders/handler-loader.ts`:
- Around line 33-38: The file uses ad-hoc error messages when validating handler
exports; replace those custom error constructions with the shared formatter
formatInvalidExportError (imported from loader-utils) to ensure consistent
messages and null-handling: locate the export validation logic in
handler-loader.ts (the blocks using getValueType, isValidExportsObject,
isValidValue and the later validation around lines ~187-193) and change any
manual error string/throw/log to call formatInvalidExportError(exportName,
expectedType, actualValue) (or the formatter's parameter order) so all
invalid-export cases use the same formatted output and correctly represent null
values.
In `@packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts`:
- Around line 33-38: Replace the ad-hoc export-error messaging in seed-loader.ts
(the blocks that validate using isValidExportsObject and isValidValue and that
call logLoadSummary) with the shared export-error formatting used by the handler
loader so null handling and message structure are consistent; specifically, when
reporting invalid exports use getValueType to produce the type part of the
message, handle null as in the handler loader, and reuse the same
phrasing/structure for the error string (also update the similar validation at
the later block around lines 172-178) so both validation sites produce identical
formatted messages.
added 2 commits
January 16, 2026 14:20
- loader-utils: Add describeValueType() helper for consistent type descriptions that properly handles null, arrays, and functions - loader-utils: Add formatInvalidValueError() for consistent value-level errors - loader-utils: Refactor formatInvalidExportError() to use describeValueType() - handler-loader: Replace ad-hoc error messages with formatInvalidExportError() and formatInvalidValueError() for consistent null handling and message structure - seed-loader: Replace ad-hoc error messages with formatInvalidExportError() and formatInvalidValueError() for consistent null handling and message structure Both loaders now produce identical formatted messages for invalid exports and values, ensuring consistent error messaging and proper null value representation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts`:
- Around line 195-226: The seed loader validates schemaName via
checkSchemaExists/findMatchingSchema but stores seeds under the original
schemaName, causing lookups to miss registry keys (e.g., 'pets' vs 'Pet');
update loadSeedFile to call findMatchingSchema(schemaName, registry) after
validation, use the returned matchedSchemaName (if non-null) as the key when
checking duplicates, logging, and when calling seeds.set, and fall back to the
original schemaName only if no match is found; adjust log messages to reference
the matched schema name and keep existing warnings when no match exists;
references: findMatchingSchema, checkSchemaExists, seeds Map, getValueType, and
logger.
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
added 2 commits
January 16, 2026 14:34
…lookups The seed loader was storing seeds under the original user-provided key (e.g., 'pets') instead of the matched registry key (e.g., 'Pet'). This caused lookups to fail when the enhancer tried to find seeds by registry schema name. Changes: - Use findMatchingSchema() directly instead of checkSchemaExists() - Store seeds under the matched schema name (if found) for proper lookups - Fall back to original name only when no registry match exists - Improve log messages to show mapping when name differs (e.g., 'pets → Pet') - Remove now-unused checkSchemaExists() function
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts (2)
380-391: Fix documentation example.The example at line 382-383 says
pluralize('pets')returns'petss', but the code correctly returns'pets'unchanged due to the early return at line 389-391. Update the documentation to match the actual behavior.📝 Suggested fix
* pluralize('pet'); // 'pets' * pluralize('order'); // 'orders' - * pluralize('pets'); // 'petss' (limitation - already plural) + * pluralize('pets'); // 'pets' (already ends in 's', returned unchanged)
402-411: Remove unreachable condition.The check
str.endsWith('s')at line 404 is dead code—it can never be true because strings ending in 's' are already handled by the early return at lines 389-391.♻️ Suggested fix
// Handle words ending in -s, -x, -z, -ch, -sh → add -es if ( - str.endsWith('s') || str.endsWith('x') || str.endsWith('z') || str.endsWith('ch') || str.endsWith('sh') ) { return `${str}es`; }
🤖 Fix all issues with AI agents
In `@packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts`:
- Around line 226-244: The exported function extractSchemaName is marked as no
longer used; either remove it to avoid dead code or explicitly preserve it as a
backward-compatible API: search for any external consumers of extractSchemaName
and if none, delete the export and the function (and remove any related
tests/docs), or if you must keep it, add a clear `@deprecated` JSDoc on
extractSchemaName, keep it exported, and add a small unit test demonstrating
current behavior so its contract stays documented; also update the module's
public API/comments accordingly.
packages/vite-plugin-open-api-server/src/loaders/seed-loader.ts
Outdated
Show resolved
Hide resolved
…e functions These functions were marked as 'no longer used' since handlers/seeds now export objects with explicit operationId/schemaName keys. Since they have no external consumers, remove them entirely instead of deprecating. Removed: - extractBaseName() from handler-loader.ts - extractSchemaName() from seed-loader.ts - Related exports from loaders/index.ts - Related unit tests (11 tests removed)
miguelramos
deleted the
fix/vite-open-api-server-thy-integrate-loaders
branch
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 by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.