WIP refactorings#686
Conversation
Greptile SummaryThis WIP PR refactors the
Confidence Score: 3/5Not safe to merge as-is: The formatter migration and Ramda refactors are mechanically sound, but
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[opapi package] --> B[Biome replaces Prettier]
A --> C[Ramda replaces lodash utils]
A --> D[openapi3-ts v2 → v4.5]
A --> E[anatine/zod-openapi v1 → v2.2]
C --> C1[state.ts: uniqBy, prop, map]
C --> C2[jsonschema.ts: R.map, R.clone]
C --> C3[handler-generator: R.map]
D --> D1[openapi.ts: openapi3-ts/oas31 subpath]
D --> D2[state.ts: SchemaObject from oas31]
D --> D3[isReferenceObject guard added]
A --> F[New features]
F --> F1[export-handler.test.ts with memfs]
F --> F2[__mocks__/fs/promises.ts]
F --> F3[createOpenapiFromSpec helper]
A --> G[WIP artifacts ⚠️]
G --> G1["foo.js — TS syntax in .js, broken imports"]
G --> G2["NewOpapi — empty exported class"]
G --> G3["createOpapiFromState — unexported dead code"]
style G1 fill:#ff6b6b,color:#fff
style G2 fill:#ffa94d,color:#fff
style G3 fill:#ffa94d,color:#fff
Reviews (1): Last reviewed commit: "tests pass" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import qs from 'qs' | ||
| import { isAxiosError } from 'axios' | ||
| import { isApiError } from './errors' | ||
| import * as types from './typings' | ||
|
|
||
| type RoutePart = | ||
| | { | ||
| type: 'argument' | ||
| name: string | ||
| } |
There was a problem hiding this comment.
foo.js uses TypeScript syntax (type annotations, generics, class access modifiers like private, public, readonly) inside a plain .js file — this is not valid JavaScript and will fail at runtime if executed without a transpiler. Additionally, it imports from ./errors and ./typings, relative paths that don't exist in the opapi package root. The content appears to be a scratch copy of the generated export-handler.ts handler code. This file should be removed before merging.
| import { exportStateAsTypescript, ExportStateAsTypescriptOptions } from './generators/ts-state' | ||
| import { generateHandler } from './handler-generator' |
There was a problem hiding this comment.
Both import lines now have a leading space. With the switch from Prettier to Biome for formatting, running pnpm check:format (biome format .) will report these lines as misformatted and fail CI. The lines should be aligned flush with the rest of the imports.
| | { | ||
| generator: 'opapi' | ||
| } | ||
| ) & | ||
| ExportStateOptions | ||
|
|
||
| export type SchemaOf<O extends OpenApi<any, any, any>> = | ||
| O extends OpenApi<infer Skema, infer _Param, infer _Sexion> ? Skema : never | ||
|
|
||
| export type ParameterOf<O extends OpenApi<any, any, any>> = | ||
| O extends OpenApi<infer _Skema, infer Param, infer _Sexion> ? Param : never | ||
|
|
||
| export type SectionOf<O extends OpenApi<any, any, any>> = | ||
| O extends OpenApi<infer _Skema, infer _Param, infer Sexion> ? Sexion : never | ||
| ) & ExportStateOptions | ||
|
|
||
| function createExportClient(state: State<string, string, string>) { | ||
| function _exportClient( | ||
| dir: string, | ||
| openapiGeneratorEndpoint: string, | ||
| postProcessors?: OpenApiPostProcessors, | ||
| ): Promise<void> | ||
| function _exportClient(dir: string, props: GenerateClientOptions): Promise<void> | ||
| function _exportClient(dir = '.', props: GenerateClientOptions | string, postProcessors?: OpenApiPostProcessors) { | ||
| let options: GenerateClientOptions | ||
| if (typeof props === 'string') { | ||
| options = { generator: 'openapi-generator', endpoint: props, postProcessors } | ||
| } else { | ||
| options = props | ||
| } | ||
|
|
||
| if (options.generator === 'openapi-generator') { | ||
| return generateClientWithOpenapiGenerator(state, dir, options.endpoint, options.postProcessors) | ||
| } | ||
| if (options.generator === 'opapi') { | ||
| return generateClientWithOpapi(state, dir) | ||
| } | ||
| throw new Error('Unknown generator') | ||
| } | ||
| return _exportClient | ||
| } | ||
|
|
||
| const createOpapiFromState = < | ||
| SchemaName extends string, | ||
| DefaultParameterName extends string, | ||
| SectionName extends string, | ||
| >( | ||
| state: State<SchemaName, DefaultParameterName, SectionName>, | ||
| ) => { | ||
| type InputItem = { | ||
| type: Parameter<'zod-schema'>['type'] | ||
| description: Parameter<'zod-schema'>['description'] | ||
| } | ||
| type Inputs = { | ||
| parameters: { | ||
| query: Record<string, InputItem> | ||
| } | ||
| response: Operation<DefaultParameterName, SectionName, string, 'zod-schema'>['response'] | ||
| } | ||
| const complexifyParameters = (parameters: Inputs['parameters']): ParametersMap<string, 'zod-schema'> => { | ||
| const map: ParametersMap<string, 'zod-schema'> = {} | ||
| for (const [key, value] of Object.entries(parameters.query)) { | ||
| map[key] = { | ||
| type: value.type, | ||
| description: value.description, | ||
| in: 'query', | ||
| } as any | ||
| } | ||
| return map | ||
| } | ||
| const simp = { | ||
| ops: { | ||
| get: (path: string, name: string, inputs: Inputs) => { | ||
| addOperation(state, { | ||
| name, | ||
| description: name, | ||
| method: 'get', | ||
| path, | ||
| parameters: complexifyParameters(inputs.parameters), | ||
| response: inputs.response, | ||
| }) | ||
| }, | ||
| }, | ||
| dt: { | ||
| boolean: (description: string): InputItem => ({ | ||
| type: 'boolean', | ||
| description, | ||
| }), | ||
| integer: (description: string): InputItem => ({ | ||
| type: 'integer', | ||
| description, | ||
| }), | ||
| number: (description: string): InputItem => ({ | ||
| type: 'number', | ||
| description, | ||
| }), | ||
| }, | ||
| } as const | ||
|
|
||
| return { | ||
| getModelRef: (name: SchemaName): OpenApiZodAny => getRef(state, ComponentType.SCHEMAS, name), | ||
| addOperation: <Path extends string>( | ||
| operationProps: Operation<DefaultParameterName, SectionName, Path, 'zod-schema'>, | ||
| ) => addOperation(state, operationProps), | ||
| exportClient: createExportClient(state), | ||
| exportTypesBySection: (dir = '.') => generateTypesBySection(state, dir), | ||
| exportServer: (dir = '.', useExpressTypes: boolean) => generateServer(state, dir, useExpressTypes), | ||
| exportOpenapi: (dir = '.') => generateOpenapi(state, dir), | ||
| exportState: (dir = '.', opts?: ExportStateAsTypescriptOptions) => exportStateAsTypescript(state, dir, opts), | ||
| exportErrors: (dir = '.') => generateErrorsFile(state.errors ?? [], dir), | ||
| exportHandler: (dir = '.') => generateHandler(state, dir), | ||
| simp, | ||
| } | ||
| } | ||
|
|
||
| export type SchemaOf<O extends OpenApi<any, any, any>> = O extends OpenApi<infer Skema, infer _Param, infer _Sexion> | ||
| ? Skema | ||
| : never | ||
|
|
||
| export type ParameterOf<O extends OpenApi<any, any, any>> = O extends OpenApi<infer _Skema, infer Param, infer _Sexion> | ||
| ? Param | ||
| : never | ||
|
|
||
| export type SectionOf<O extends OpenApi<any, any, any>> = O extends OpenApi<infer _Skema, infer _Param, infer Sexion> | ||
| ? Sexion | ||
| : never | ||
|
|
||
| export { exportJsonSchemas, exportZodSchemas } from './handler-generator/export-schemas' |
There was a problem hiding this comment.
createOpapiFromState and NewOpapi are unreachable dead code
createOpapiFromState is defined but never exported or called anywhere in the codebase. Similarly, NewOpapi is exported but contains only an empty state: {} with no methods. Both appear to be early-stage WIP sketches. Dead code in a non-WIP commit adds maintenance burden and confuses callers about the intended API surface. These should either be completed, hidden behind a feature flag, or removed before merging.
| normalizePath: (path, { req }) => normalizePath()(path, { req }), | ||
| metricBuckets: { |
There was a problem hiding this comment.
normalizePath() factory is invoked on every request
The previous code called normalizePath() once at middleware setup time and passed the resulting function as the normalizePath option. The new code wraps it in a lambda that calls normalizePath() on every single request, allocating a new closure each time. Because normalizePath() itself is stateless (it only closes over the module-level appRoutes map), the behavior is equivalent, but the allocation is unnecessary. Consider calling normalizePath() once:
normalizePath: normalizePath(),1 participant
WIP