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

šŸ“£ v6.0 RFC and call for contributors #961

Closed
7 tasks done
drwpow opened this issue Oct 30, 2022 · 11 comments
Closed
7 tasks done

šŸ“£ v6.0 RFC and call for contributors #961

drwpow opened this issue Oct 30, 2022 · 11 comments
Labels
3.1 Part of OpenAPI 3.1 enhancement New feature or request openapi-ts Relevant to the openapi-typescript library planned Expected in an upcoming version

Comments

@drwpow
Copy link
Contributor

drwpow commented Oct 30, 2022

šŸ‘Æā€ā™‚ļø v6.0

openapi-typescript has an upcoming 6.0 version, which started as a breaking change but has now expanded to include the following as well:

šŸ¤ Call for contributors

This library would love to have more contributors and maintainers help keep this project going! In service of this, Iā€™ve recently added:

  • Issue templates for better bug filing
  • Improved contributor docs to make contributions friendlier
  • Codebase cleanup that makes onboarding simpler.

If youā€™ve been interested in contributing to opensource, or if youā€™ve found this tool useful at your workplace or personal projects and would love to contribute, please raise a hand āœ‹! Anyone of any skill level and experience with the project is welcome šŸ™‚

Note that at this time, contributing monetarily isnā€™t necessary. Perhaps in the future, but for now this library is held back by time constraints and number of maintainers.

ā“ v6.0 RFC

Would love some šŸ‘ or šŸ‘Ž (comments welcome) on the following changes for 6.0 and beyond:

ā“ 1. Dropping Swagger 2.x support

Swagger 3.0 has been out for 5 years now and already many (most?) APIs have fully made the leap to 3.x. Anecdotally I havenā€™t seen an active 2.0 schema in the wild for quite some time. Dropping support for 2.x would shed quite a bit of dead weight in the codebase and allow for faster releases. Of course, users of Swagger 2.x could always use openapi-typescript@5 indefinitely to generate types for their 2.x schemas, but wonā€™t receive any future updates.

ā“ 2. Dropping Prettier

Prettier was originally added as a lazy way to clean up the generated output. But with the in-progress code improvements, the output is much better formatted to the point Prettier isnā€™t needed (to be clear, the output doesnā€™t and never will match Prettier 100%, but itā€™s still formatted neatly to the point Prettier isnā€™t needed). Dropping Prettier would mean much faster generation (initial testing shows a 3Ɨ speedup on large schemas) and it would also shed this libraryā€™s weight by quite a bit. And users could use their formatting method of choice (#959).

ā“ 3. Keeping string manipulation (over AST transformation)

There are 2 types of codegen libraries: those that use string manipulation, and those that use AST transformation. This library uses the former initially out of expediency, but over the years has stuck with this pattern for ease of maintenance and performance. Even though thereā€™s no change here, the idea of switching to TypeScriptā€™s AST has been raised before, and I wanted to raise it again. I wonā€™t explain the pros/cons of each here, because if no one reading is familiar with how ASTs work I think Iā€™ve made my point and ASTs would be harder to maintain šŸ˜‰. But regardless, thoughts are welcome for those that have them here.

ā“ 4. [Insert your idea here]

If youā€™d like to propose something not mentioned here, either as a breaking change or as a feature request, please speak now! For ideas, you can look back at suggested enhancements that havenā€™t been implemented yet.


šŸ™ Thanks so much for all the support youā€™ve given this library. I canā€™t believe a simple little tool we made at a past company has resulted in millions of downloads and is used in thousands of projects. Thanks for all the PRs, bugfixes, and passionate contributions made by this community!

Hereā€™s to an even better, and faster v6 (and beyond) šŸ˜Ž

@drwpow drwpow added enhancement New feature or request 3.1 Part of OpenAPI 3.1 planned Expected in an upcoming version labels Oct 30, 2022
@drwpow drwpow pinned this issue Oct 30, 2022
@drwpow
Copy link
Contributor Author

drwpow commented Nov 4, 2022

Update: this is largely done in #968. Once that merges, itā€™ll just take some testing and possibly adding some more tests (many tests were streamlined, and many, many tests were combined wherever possible).

@drwpow
Copy link
Contributor Author

drwpow commented Nov 9, 2022

v6.0.0 shipped! šŸŽ‰

The call for contributors is still open. Any bug testing and PRs fixing bugs are welcome!

@denosaurtrain
Copy link

Nice work, @drwpow! I've been trying out v6 recently, but I used Deno which added a bunch more confusion for me, so I don't have much helpful feedback for you. Just want to say thanks for the good work!

@yacinehmito
Copy link
Contributor

Raising a hand šŸ¤š
(that doesn't exist as a reaction, sorry)

I plan to use openapi-typescript extensively at our company. I have a keen interest in making the library portable to other runtimes (browser, Deno, Cloudflare Workers), without compatibility layers.

@npbenjohnson
Copy link

šŸ‘ Use AST - With a utility library like ts-morph, and an AST tool like https://ts-ast-viewer.com/, I think using AST would actually lower the barrier to entry for contributing to this project. It is a lot more effort to understand the branching generation logic in this library, and find a reasonable place to extend it if the injection point required is not already a point being filled in. If it used AST, making a modification could just be selecting the correct node(s) on the existing output and making the modification. It would also allow for more easily composing structures that may be created in more than 1 pass, like for updating jsdoc comments when the related structure changes.

@rxliuli
Copy link

rxliuli commented Dec 14, 2022

AST is actually not as complicated as imagined, you can look at ast-types/recast, their abstraction of AST is very elegant, much simpler than the AST of typescript compiler, it is just a constantly nested JSON object, you can use JS recursion Traverse or use tools to query specified nodes.
astexplorer allows viewing the ast structure, again, it's much simpler
https://astexplorer.net/

ast-types example

const ast = b.exportNamedDeclaration(
  b.tsInterfaceDeclaration(
    b.identifier('User'),
    b.tsInterfaceBody([
      b.tsPropertySignature(
        b.identifier('name'),
        b.tsTypeAnnotation(b.tsStringKeyword()),
      ),
      b.tsPropertySignature(
        b.identifier('age'),
        b.tsTypeAnnotation(b.tsNumberKeyword()),
      ),
    ]),
  ),
)

ts api example

factory.createInterfaceDeclaration(
  undefined,
  [factory.createModifier(ts.SyntaxKind.ExportKeyword)],
  factory.createIdentifier('User'),
  undefined,
  undefined,
  [
    factory.createPropertySignature(
      undefined,
      factory.createIdentifier('name'),
      undefined,
      factory.createKeywordTypeNode(ts.SyntaxKind.StringKeyword),
    ),
    factory.createPropertySignature(
      undefined,
      factory.createIdentifier('age'),
      undefined,
      factory.createKeywordTypeNode(ts.SyntaxKind.NumberKeyword),
    ),
  ],
)

However, I even wrote a generator that uses ast-types to generate code, e.g.

input

export interface User {
  name: string
  age: number
}

output

b.exportNamedDeclaration(
  b.tsInterfaceDeclaration(
    b.identifier('User'),
    b.tsInterfaceBody([
      b.tsPropertySignature(b.identifier('name'), b.tsTypeAnnotation(b.tsStringKeyword())),
      b.tsPropertySignature(b.identifier('age'), b.tsTypeAnnotation(b.tsNumberKeyword())),
    ])
  )
)

ref: https://gist.github.com/rxliuli/d5286a1ea6d028c25e7b26c338ceac58

@benevbright
Copy link
Contributor

Is it currently possible to get paths as an object for runtime?

@drwpow drwpow mentioned this issue Jan 21, 2023
3 tasks
@drwpow
Copy link
Contributor Author

drwpow commented Feb 23, 2023

@npbenjohnson @rxliuli you both make some compelling arguments for using AST! While it probably wouldnā€™t change the overarching logic of this codebase a whole lot, if you feel it would lower the barrier to entry Iā€™d be all for it. Having written quite a few Babel transformers myself, and talking to the maintainers, I just think about how hard it was for that project to write docs or get contributors because of the complexity.

But on the other hand, transformation is transformation, and expressing the problem in a syntax meant for transformation isnā€™t a complexity add; quite the opposite.

I do like ast-types/recastā€™s syntax, as you mentioned, @rxliuli. Agreed it looks simpler than TypeScriptā€™s native syntax. Perhaps thereā€™s some interesting approach we could take that doesnā€™t inherit TSā€™ weirdness or slowness when it comes to compile. Because love or hate this libraryā€™s current approach of string mashing, at least itā€™s fast! šŸ˜„

@mitchell-merry
Copy link
Contributor

Officially raising my hand āœ‹šŸ»!

@sregg
Copy link

sregg commented May 18, 2023

6.0 has been out for 6 months. Should this be still open?

@drwpow drwpow added the openapi-ts Relevant to the openapi-typescript library label May 22, 2023
@drwpow
Copy link
Contributor Author

drwpow commented May 22, 2023

The call for contributors is still open! But to your point, I can update the messaging so itā€™s more clear. Until I open another issue, I can close & unpin this.


An update on the AST discussion from this thread: I received so many wonderful comments and ideas about that approach.

Short explanation: Weā€™ll keep the library producing strings for the foreseeable future.

Longer explanation: Iā€™m still open to the idea of an AST, but this libraryā€™s current form is not only as performant as it will ever be, but also will never block any feature from landing (because you can always produce a freeform string wherever you want). An AST will always slow down performance, but in return will yield higher accuracy. As of right now, this library doesnā€™t struggle with producing invalid TS (or, rather, the bugs it does encounters are from complex schema types, which would still present itself in an AST as well). While this library is not solely mineā€”most of the work has been done by contributorsā€”speaking solely for myself I place a high emphasis over performance (6.x of this library, for those that hadnā€™t noticed, got an order of magnitude speedup). And as of now I would have to spend a lot more time investigating an AST approach that would bring about a dramatically-improved contributor experience without sacrificing performance.

@drwpow drwpow closed this as completed May 22, 2023
@drwpow drwpow unpinned this issue May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.1 Part of OpenAPI 3.1 enhancement New feature or request openapi-ts Relevant to the openapi-typescript library planned Expected in an upcoming version
Projects
None yet
Development

No branches or pull requests

8 participants