Skip to content
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

feat: experimental typescript-nextjs template #152

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

mnahkies
Copy link
Owner

@mnahkies mnahkies commented Apr 20, 2024

Creates a new typescript-nextjs template

  • Server only so far, plan to add client
  • Generates routes following the app router "Route handlers", with a mirrored directory structure of companion files under ./src/generated containing the types and validation boilerplate

Approach

Due to the NextJS file based routing paradigm I couldn't see anyway to avoid manipulating files that will contain manually written code.

  • This is a major shift compared to the other templates which treat all emitted files as disposable.
  • To achieve this we employ ts-morph to (hopefully) safely modify the app router files without destroying the manually written implementations.
  • Plan to implement require clean git working directory by default #52 before releasing this, I remember early linting tools mangling/damaging my code, and there's a pretty high chance the morphing will have edge cases I've missed

Pending Refactoring

Currently there is a lot of duplicated code between this and the typescript-koa template which needs rationalizing, as well as it depending on the typescript-koa-runtime package. 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-runtime hack, to actually use this in a NextJS application you need to add the following to your next.config.mjs:

webpack: (
    config,
    {buildId, dev, isServer, defaultLoaders, nextRuntime, webpack}
  ) => {
    config.resolve.alias = {
      ...config.resolve.alias,
      koa: false,
      '@koa/cors': false,
      'koa-body': false,
    }
    // Important: return the modified config
    return config
  },

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-morph can be relatively slow, so it's probably useful to check the performance between this and typescript-koa

Overall 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

[info] elapsed {
  total: '3735 ms',
  '0 - program starting': '0 ms, 0%',
  '1 - load files': '292 ms, 8%',
  '2 - calculate schema dependency graph': '1528 ms, 41%',
  '3 - generate ./generated/api.github.com.yaml/models.ts': '268 ms, 7%',
  '4 - generate ./generated/api.github.com.yaml/schemas.ts': '314 ms, 8%',
  '5 - format output': '1128 ms, 30%',
  '6 - write output': '49 ms, 1%'
}

typescript-koa - api.github.com.yaml

[info] elapsed {
  total: '2398 ms',
  '0 - program starting': '0 ms, 0%',
  '1 - load files': '293 ms, 12%',
  '2 - calculate schema dependency graph': '506 ms, 21%',
  '3 - generate ./models.ts': '267 ms, 11%',
  '4 - generate ./schemas.ts': '322 ms, 13%',
  '5 - format output': '851 ms, 35%',
  '6 - write output': '4 ms, 0%'
}

@mnahkies mnahkies force-pushed the mn/feat/typescript-nextjs branch 3 times, most recently from 54335e8 to 3033f06 Compare April 27, 2024 08:12
mnahkies added a commit to mnahkies/openapi.tools that referenced this pull request Jul 28, 2024
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.
@mnahkies mnahkies force-pushed the mn/feat/typescript-nextjs branch from 3033f06 to 58f706b Compare August 26, 2024 10:49
@mnahkies mnahkies force-pushed the mn/feat/typescript-nextjs branch from fd15300 to 258f9ab Compare September 20, 2024 11:09
@mnahkies mnahkies force-pushed the mn/feat/typescript-nextjs branch from 258f9ab to aedad74 Compare December 22, 2024 10:59
paths.includes(relative),
)

return alias ? alias[0].replace("*", "") : undefined

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This replaces only the first occurrence of "*".

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that all occurrences of the asterisk (*) are replaced in the string. This can be achieved by using a regular expression with the global flag (g). This change will ensure that all instances of the asterisk are replaced, not just the first one.

The specific change will be made in the findImportAlias function, where the replace method is used. We will replace the current replace call with a call to replace that uses a regular expression with the global flag.

Suggested changeset 1
packages/openapi-code-generator/src/typescript/typescript-nextjs/typescript-nextjs.generator.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/openapi-code-generator/src/typescript/typescript-nextjs/typescript-nextjs.generator.ts b/packages/openapi-code-generator/src/typescript/typescript-nextjs/typescript-nextjs.generator.ts
--- a/packages/openapi-code-generator/src/typescript/typescript-nextjs/typescript-nextjs.generator.ts
+++ b/packages/openapi-code-generator/src/typescript/typescript-nextjs/typescript-nextjs.generator.ts
@@ -468,3 +468,3 @@
 
-  return alias ? alias[0].replace("*", "") : undefined
+  return alias ? alias[0].replace(/\*/g, "") : undefined
 }
EOF
@@ -468,3 +468,3 @@

return alias ? alias[0].replace("*", "") : undefined
return alias ? alias[0].replace(/\*/g, "") : undefined
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants