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!: changing openapi generator #50

Merged
merged 5 commits into from
Apr 9, 2024

Conversation

seriouslag
Copy link
Collaborator

@seriouslag seriouslag commented Mar 23, 2024

Changed openapi code generator to @hey-api/openapi-ts.

Note: @hey-api/openapi-ts is rapidly changing its API.
We should consider that before merging to main.

Supporting new properties:

  • --base
  • --serviceResponse
  • --enums
  • --operationId
  • --lint
  • --format
  • --useDateType (see below)

BREAKING CHANGE:
--indent, --postfixModels, --postfixServices and --useUnionTypes properties were removed in @hey-api/openapi-ts.
Removed them from this API.

BREAKING CHANGE:
I converted this library to a Javascript module, setting the type module in the package.json and changing TSC to compile to NodeNext. This is not a breaking change for most consumers of this package, but it could be for older node versions.

This was needed to build with @hey-api/openapi-ts.

Closes: #42

I tested the --useDateType property. It works as specified in the @hey-api/openapi-ts library.
However, it only changes the generated model to a JS Date object and doesn't convert the string to a Date object.
As the model doesn't match the data.
I have opened an issue in that library.

Note: I added a new library to help traverse TS source files, ts-morph

@AnderssonPeter
Copy link

Will this pr also enable the date type? If I remember correctly they somehow converted dates to the JavaScript type Date, I'm a bit unsure if that happened automatically or if you had to enable it.

@seriouslag
Copy link
Collaborator Author

seriouslag commented Mar 23, 2024

Will this pr also enable the date type? If I remember correctly they somehow converted dates to the JavaScript type Date, I'm a bit unsure if that happened automatically or if you had to enable it.

The property can be enabled, but I still need to test it. If you can, please pull this branch and test your use case.

@AnderssonPeter
Copy link

Once this is merged and a new release has been created, then i could test it.

@AnderssonPeter
Copy link

Will this pr also enable the date type? If I remember correctly they somehow converted dates to the JavaScript type Date, I'm a bit unsure if that happened automatically or if you had to enable it.

The property can be enabled, but I still need to test it. If you can, please pull this branch and test your use case.

I tried to run it but I'm getting compilation errors, I'm a bit unsure how to run it from source..
I modified examples\react-app\package.json -> generate:api to be node ../../dist/cli.js --useDateType -i ./swagger.json -o output.

But when i run npm run preview in the root, i just get the error

import { generate } from "./generate.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module

@seriouslag
Copy link
Collaborator Author

seriouslag commented Mar 26, 2024

Will this pr also enable the date type? If I remember correctly they somehow converted dates to the JavaScript type Date, I'm a bit unsure if that happened automatically or if you had to enable it.

The property can be enabled, but I still need to test it. If you can, please pull this branch and test your use case.

I tried to run it but I'm getting compilation errors, I'm a bit unsure how to run it from source.. I modified examples\react-app\package.json -> generate:api to be node ../../dist/cli.js --useDateType -i ./swagger.json -o output.

But when i run npm run preview in the root, i just get the error

import { generate } from "./generate.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module

Thank you for testing it out.
What version of node/npm are you using?

To run from source:

  • pull branch
  • pnpm install
  • change the openapi spec file (as it seems you did) in the example/react-app package.json
  • npm run preview in root folder of project

It appears your source built and is failing on the generate:api step of example/react-apps package.json.

@seriouslag
Copy link
Collaborator Author

seriouslag commented Mar 26, 2024

@AnderssonPeter I tested the useDateType property.
It works as specified in the @hey-api/openapi-ts library.
However, it only changes the generated model property to a JS Date object type and doesn't convert the string to a Date object.
As the model doesn't match the data, I see this as useless and incorrect.
I have opened an issue in that library.

@seriouslag seriouslag force-pushed the major/change-underlying-api-generator branch from 5d7cd9c to 725ddc8 Compare March 27, 2024 14:50
Copy link
Owner

@7nohe 7nohe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seriouslag That's excellent work. Thank you very much.
I would appreciate it if you could also make changes to the part on line 9 of the README.

@AnderssonPeter
Copy link

Will this pr also enable the date type? If I remember correctly they somehow converted dates to the JavaScript type Date, I'm a bit unsure if that happened automatically or if you had to enable it.

The property can be enabled, but I still need to test it. If you can, please pull this branch and test your use case.

I tried to run it but I'm getting compilation errors, I'm a bit unsure how to run it from source.. I modified examples\react-app\package.json -> generate:api to be node ../../dist/cli.js --useDateType -i ./swagger.json -o output.

But when i run npm run preview in the root, i just get the error

import { generate } from "./generate.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module

Thank you for testing it out.
What version of node/npm are you using?

To run from source:

  • pull branch
  • pnpm install
  • change the openapi spec file (as it seems you did) in the example/react-app package.json
  • npm run preview in root folder of project

It appears your source built and is failing on the generate:api step of example/react-apps package.json.

I'm fairly certain I did the steps you listed, I ran it on node 18 I think, but not 100 sure as I'm not at my PC.

@7nohe 7nohe assigned 7nohe and seriouslag and unassigned 7nohe Mar 30, 2024
@seriouslag seriouslag force-pushed the major/change-underlying-api-generator branch from 725ddc8 to 85999aa Compare March 30, 2024 17:35
@seriouslag
Copy link
Collaborator Author

@seriouslag That's excellent work. Thank you very much. I would appreciate it if you could also make changes to the part on line 9 of the README.

Updated readme. Do we want to merge with the underlying client being below version one?

@seriouslag
Copy link
Collaborator Author

Hey all, we plan to provide a similar functionality (albeit with a different API) in the future in https://github.com/hey-api/openapi-ts. If you'd like to work on that package, feel free to reach out to me!

What functionality?

@seriouslag
Copy link
Collaborator Author

seriouslag commented Apr 1, 2024

Thoughts @seriouslag ?

An active PR is not a good place to self-promote your thoughts about planned functionality in a different project.
I want to collaborate, and I am sure others would as well.
Feel free to start working on your implementation if you think you can do a better job than what this project has done, and then open an issue in this repo if you care to.

@mrlubos
Copy link

mrlubos commented Apr 1, 2024

Gotcha @seriouslag. I will delete my previous comments, just wanted to get a sense on the idea. Thanks for your feedback!

@seriouslag
Copy link
Collaborator Author

Will this pr also enable the date type? If I remember correctly they somehow converted dates to the JavaScript type Date, I'm a bit unsure if that happened automatically or if you had to enable it.

The property can be enabled, but I still need to test it. If you can, please pull this branch and test your use case.

I tried to run it but I'm getting compilation errors, I'm a bit unsure how to run it from source.. I modified examples\react-app\package.json -> generate:api to be node ../../dist/cli.js --useDateType -i ./swagger.json -o output.
But when i run npm run preview in the root, i just get the error

import { generate } from "./generate.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module

Thank you for testing it out.
What version of node/npm are you using?
To run from source:

  • pull branch
  • pnpm install
  • change the openapi spec file (as it seems you did) in the example/react-app package.json
  • npm run preview in root folder of project

It appears your source built and is failing on the generate:api step of example/react-apps package.json.

I'm fairly certain I did the steps you listed, I ran it on node 18 I think, but not 100 sure as I'm not at my PC.

I was able to duplicate the issue; I now converted all files to mjs, which I believe will fix this issue.
I would appreciate it if you could pull the latest and try again.

@seriouslag seriouslag force-pushed the major/change-underlying-api-generator branch from 9777225 to e09fc02 Compare April 7, 2024 16:48
Changed openapi code generator to @hey-api/openapi-ts.

Not supporting all properties yet.

Supporting new properties:
- base
- serviceResponse
- enums
- useDateType

indent and useUnionTypes properties were removed
left in for backwards compatibility.

BREAKING CHANGE: changed from cjs to mjs
@seriouslag seriouslag force-pushed the major/change-underlying-api-generator branch from e09fc02 to 621f2b6 Compare April 8, 2024 16:59
@seriouslag
Copy link
Collaborator Author

Updated PR to support the --useOptions of @hey-api/openapi-ts.
Also dropped all deprecated options.

- removed deprecated options of @hey-api/openapi-ts
- using ts-morph to help traverse typescript source files
@seriouslag seriouslag force-pushed the major/change-underlying-api-generator branch from 621f2b6 to baafa2d Compare April 8, 2024 17:10
@nopitown
Copy link
Sponsor Contributor

nopitown commented Apr 8, 2024

👏🏼 👏🏼 👏🏼 👏🏼 Looking forward this new generator.

@7nohe
Copy link
Owner

7nohe commented Apr 9, 2024

It's working nicely on my PC, so let's go ahead and merge this and release it!

@7nohe 7nohe merged commit a57cae5 into main Apr 9, 2024
4 checks passed
@7nohe 7nohe deleted the major/change-underlying-api-generator branch April 9, 2024 21:59
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.

Upgrade to v.0.26.0 of openapi-typescript-codegen
5 participants