-
Notifications
You must be signed in to change notification settings - Fork 5
feat: experimental typescript-nextjs template #152
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
base: main
Are you sure you want to change the base?
Conversation
54335e8 to
3033f06
Compare
adds https://github.com/mnahkies/openapi-code-generator/ / https://openapi-code-generator.nahkies.co.nz/ to the list. it currently supports generating typescript client sdks based on fetch / axios, and server routing / request+response validation based on koa (choice of zod / joi for runtime validation). an experimental nextjs server implementation is in the works (mnahkies/openapi-code-generator#152), and my longer term plan is to add other languages such as kotlin to the set of templates.
3033f06 to
58f706b
Compare
fd15300 to
258f9ab
Compare
258f9ab to
aedad74
Compare
packages/openapi-code-generator/src/typescript/typescript-nextjs/typescript-nextjs.generator.ts
Fixed
Show fixed
Hide fixed
9603fcd to
c47b5cb
Compare
c47b5cb to
2e6b6f2
Compare
| paths.includes(relative), | ||
| ) | ||
|
|
||
| return alias ? alias[0].replace("*", "") : undefined |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 24 hours ago
In general, to fix incomplete string escaping or encoding when using String.prototype.replace, you should either (a) use a regular expression with the g flag so that all occurrences of the pattern are replaced, or (b) use an API that is explicitly global (like replaceAll). For simple literal patterns such as "*", converting to a global regular expression is straightforward.
In this specific case, the best way to fix the problem without changing existing functionality is to update alias[0].replace("*", "") so that every * in the alias key is removed, not just the first. The simplest change is to use a global regular expression: alias[0].replace(/\*/g, ""). This continues to turn a typical alias like "@app/*" into "@app/", but also handles any aliases that might have more than one asterisk. No additional imports or helper methods are required; this is a built-in JavaScript feature. The change is localized to the findImportAlias function in packages/openapi-code-generator/src/typescript/server/typescript-nextjs/typescript-nextjs.generator.ts, line 22 in the snippet.
-
Copy modified line R22
| @@ -19,7 +19,7 @@ | ||
| paths.includes(relative), | ||
| ) | ||
|
|
||
| return alias ? alias[0].replace("*", "") : undefined | ||
| return alias ? alias[0].replace(/\*/g, "") : undefined | ||
| } | ||
|
|
||
| export async function generateTypescriptNextJS( |
fa2f9dc to
f37745b
Compare
introduces a second server template, `typescript-express` **Tasks** - [x] Template - [x] Runtime - [x] Integration tests - [x] Existing E2E tests - [x] Additional E2E tests - [x] Documentation - [x] Review **Why Express** `express` is one of the most popular server frameworks for NodeJS, getting approximately 10x as many weekly downloads as `koa` Its also quite similar to `koa` making it a good candidate for the second server template. This makes it somewhat less interesting to implement, compared to other options like `nextjs` (#152) but also means that its a good way to prompt refactors like #316 without requiring a bunch of new functionality. **Runtime** It's clear at this point that there is a lot of duplicated code between all the runtimes. I like keeping the separate runtime packages for several reasons: - Dependency declaration / management is cleaner - NPM download trends can function as a rough proxy for adoption / usage _(although private caching registries cause this to under-report significantly)_ It probably makes sense to introduce a `typescript-runtime-common` package soon. **Approach** The approach ends up looking near identical to the `typescript-koa` template, in terms of the end developer experience. This is particularly reinforced by the E2E tests and how little difference exists between the two implementations.
a1a0a6a to
b075b59
Compare
- upgrade all dependencies - regenerate with latest nextjs generator from mnahkies/openapi-code-generator#152 - adjust for nextjs 15
b075b59 to
1876c5e
Compare
f61f814 to
3684213
Compare
required in order to use the latest version of `ts-morph` over in #152
da3921c to
36635f0
Compare
1bb2116 to
8974ef3
Compare
| params.body.schema | ||
| ? `parseRequestInput(${params.body.schema}, await request.json(), RequestInputType.RequestBody)${!params.body.isSupported ? " as never" : ""}` | ||
| : "undefined" |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Code construction depends on an
improperly sanitized value
Code construction depends on an
improperly sanitized value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 24 hours ago
General fix: ensure that the string obtained from JSON.stringify(model.default) is further sanitized before being interpolated into generated code, so that characters like <, >, /, \u2028, \u2029, etc., cannot break out of their intended context if the generated code is ever embedded in HTML or similar sinks.
Best concrete fix for this codebase:
- Introduce a small, shared escaping helper in
AbstractSchemaBuilderthat takes the JSON string and replaces potentially dangerous characters with safe escape sequences, following CodeQL’s recommended mapping (e.g.<→\u003C,>→\u003E,/→\u002F,\u2028/\u2029→\u2028/\u2029escape forms). - Use this helper in the
defaultmethods ofJoiBuilder,ZodV3Builder, andZodV4Builderinstead of usingJSON.stringify(model.default)raw. - Keep the logic that wraps
defaultValuein extra quotes for string‑typed models unchanged, but apply it to the sanitized string. SinceJSON.stringifyalready returns a valid JavaScript literal, and the additional escaping only introduces further backslash escapes, the semantics of the default values remain the same.
Specifically:
- In
abstract-schema-builder.ts, add aprotected escapeUnsafeJsonString(value: string): stringmethod that implements the CodeQL‑style escaping on top of the JSON string. - In
joi-schema-builder.ts, change thedefaultimplementation so that it computesconst defaultValue = this.escapeUnsafeJsonString(JSON.stringify(model.default)). - Do the same in
zod-v3-schema-builder.tsandzod-v4-schema-builder.ts.
No changes are required in server-operation-builder.ts or typescript-nextjs-router-builder.ts themselves; once the schema builders sanitize the default value, the tainted flow into the router template will be safe.
-
Copy modified lines R337-R339
| @@ -334,7 +334,9 @@ | ||
| typeof model.default !== "string" && | ||
| !(model.nullable && model.default === null) | ||
|
|
||
| const defaultValue = JSON.stringify(model.default) | ||
| const defaultValue = this.escapeUnsafeJsonString( | ||
| JSON.stringify(model.default), | ||
| ) | ||
|
|
||
| return [ | ||
| schema, |
-
Copy modified lines R379-R381
| @@ -376,7 +376,9 @@ | ||
| model.type === "string" && | ||
| typeof model.default !== "string" && | ||
| !(model.nullable && model.default === null) | ||
| const defaultValue = JSON.stringify(model.default) | ||
| const defaultValue = this.escapeUnsafeJsonString( | ||
| JSON.stringify(model.default), | ||
| ) | ||
|
|
||
| return [ | ||
| schema, |
-
Copy modified lines R370-R372
| @@ -367,7 +367,9 @@ | ||
| model.type === "string" && | ||
| typeof model.default !== "string" && | ||
| !(model.nullable && model.default === null) | ||
| const defaultValue = JSON.stringify(model.default) | ||
| const defaultValue = this.escapeUnsafeJsonString( | ||
| JSON.stringify(model.default), | ||
| ) | ||
|
|
||
| return [ | ||
| schema, |
-
Copy modified lines R401-R428
| @@ -398,6 +398,34 @@ | ||
|
|
||
| protected abstract default(schema: string, model: IRModelBase): string | ||
|
|
||
| /** | ||
| * Escape characters in a JSON string that can cause issues when embedded | ||
| * into generated code, particularly when that code is inserted into HTML. | ||
| * | ||
| * This should be used on top of JSON.stringify(model.default). | ||
| */ | ||
| protected escapeUnsafeJsonString(value: string): string { | ||
| const charMap: Record<string, string> = { | ||
| "<": "\\u003C", | ||
| ">": "\\u003E", | ||
| "/": "\\u002F", | ||
| "\\": "\\\\", | ||
| "\b": "\\b", | ||
| "\f": "\\f", | ||
| "\n": "\\n", | ||
| "\r": "\\r", | ||
| "\t": "\\t", | ||
| "\0": "\\0", | ||
| "\u2028": "\\u2028", | ||
| "\u2029": "\\u2029", | ||
| } | ||
|
|
||
| return value.replace( | ||
| /[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g, | ||
| (ch) => charMap[ch] ?? ch, | ||
| ) | ||
| } | ||
|
|
||
| public abstract void(): string | ||
|
|
||
| public abstract never(): string |
Creates a new
typescript-nextjstemplate./src/generatedcontaining the types and validation boilerplateApproach
Due to the NextJS file based routing paradigm I couldn't see anyway to avoid manipulating files that will contain manually written code.
Pending Refactoring
Currently there is a lot of duplicated code between this and the
typescript-koatemplate which needs rationalizing, as well as it depending on thetypescript-koa-runtimepackage. Part of the motivation behind this is to have more than one server template to allow it to become more generic like the client templates.Because of the
typescript-koa-runtimehack, to actually use this in a NextJS application you need to add the following to yournext.config.mjs:I also don't love the explosion of files this creates when running the standard set of integration suites, and might limit the scope down to one spec to reduce the noise. Ideally I'd like to separate the integration tests to their own repo and also start testing more permutations of options (eg:
joi,extract-inline-schemas).Additionally to make multiple suites play nicely I've had to fudge the output paths in the tests, technically producing incorrect routes.
Performance
I've been told that
ts-morphcan be relatively slow, so it's probably useful to check the performance between this andtypescript-koaOverall it looks similar, aside from calculation of the dependency graph. I'm not sure if this is the bug in the timing code, or if the larger number of compilation units is somehow causing this. Warrants some investigation in case we're calculating it repeatedly or something.
typescript-nextjs - api.github.com.yaml
typescript-koa - api.github.com.yaml