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

Generating implementation based on tags #111

Open
ADRFranklin opened this issue Nov 13, 2023 · 7 comments
Open

Generating implementation based on tags #111

ADRFranklin opened this issue Nov 13, 2023 · 7 comments

Comments

@ADRFranklin
Copy link

Having the ability to separate routes based on tags specified in the openapi schema. This would make it easier to implement routes in classes for that specific tag.

I would assume it might end up looking something like this

export type MiscEndpoints = {
  getVersion: GetVersion,
  getSpec: GetSpec,
}

export type Implementation {
  miscRoutes: MiscEndpoints ,
  // non tagged endpoint
  getInfo: GetInfo,
}

I assume this might end up requiring a little more work as to figure out that MiscRoutes implements sub routes, so I'm not sure of what the best way to do that is for this project, but hopefully this is enough information to give you an idea of what I am trying to get at.

Generally in most cases you end up having a file full of functions that implements something like /users endpoint, and you would return a router implementing those routes, but would be nice to be able to implement these routes from a class such as in my case.

Right now it's not very easy to implement from a class, cause I generally have to make another structure and target specific routes for those specific routes such as /users, and /version and /spec are misc routes.

Would like to discuss more on this topic, if you have any ideas?

@mnahkies
Copy link
Owner

mnahkies commented Dec 3, 2023

I'm certainly interested in exploring ideas like this, generally speaking there's a few different approaches we could take to grouping:

  • Path segments
  • Tags
  • A custom property like x-group-name or something

With path segments, there's a question of how many levels to descend for the grouping, eg:

/foo/bar/baz
/foo/foobar/bla

Do we create a single group for /foo/* or a group for /foo/bar and /foo/foobar? Or do we make this configurable? What if I want to group by the first two segments of some paths but not others.

For tags there is a similar issue if there is more than one tag, though perhaps you just assume the first tag is the group.

Implementing this kind of grouping staying with-in the single file output that currently exists probably isn't too hard.

Ideally I'd like to also explore outputting into several files instead of a single giant file, but this gets tricky as you then have to get all the imports correct.

@mnahkies
Copy link
Owner

mnahkies commented Mar 3, 2024

Initial experimental support for this has been merged in #134 - it's pretty rudimentary still, and will probably get some breaking changes, but should be a bit quicker to iterate on now that the initial refactoring has been done.

@ADRFranklin
Copy link
Author

ADRFranklin commented Mar 4, 2024

@mnahkies so far this has been great, just a few things that I would like to see for it though, one being that the name of the 'Implementation' renamed to match the tag so 'AuthImplementation' as currently when importing all the routes implementations, I have to rename the import, and converting from 'type' to either 'interface' or better for me 'abstract class' so that I am able to register the routes in a class that implements the 'Implementation' and pass it to DI (The DI I use makes use of abstract classes https://github.com/artberri/diod).

I'm unaware of a way to convert a type to an interface/abstract class, at least in a way that is easily maintainable that is, so I am assuming this is the best method for me.

Edit: One other thing was having a global interface that uses the type/interface of the routes to ensure nothing is missed, as of right now, I could implement 90% of the route groups and forget to implement one of the route groups and nothing would error or complain at me to implement it, where as with a global interface if it wasn't implement it would complain.

@mnahkies
Copy link
Owner

mnahkies commented Mar 7, 2024

Great feedback thanks - I was already thinking about the naming point, just hadn't had a chance to do this yet. I'm planning to add this very soon, though I'm trying to work out if there's a good way to expose the grouping and naming criteria in a really flexible way on the cli

I'd been thinking about the type Vs interface point, but not really the idea of doing abstract class - will have a play, but I suspect some cli parameter to choose between the three would be relatively straightforward and probably make sense

@ADRFranklin
Copy link
Author

I'd been thinking about the type Vs interface point, but not really the idea of doing abstract class - will have a play, but I suspect some cli parameter to choose between the three would be relatively straightforward and probably make sense

Any chance you have had time to think about these options? I'd really love to be able to make use of the abstract class, rather then having to write wrappers around the generated types to make it work with my dependency injection library.

@mnahkies
Copy link
Owner

Hwy @ADRFranklin, sorry this fell off my radar.

Here's a draft implementation it would be great to get your thoughts on: #256 (see mnahkies/node-scim#1 showing it being used in practice with diod, feel free to leave comments on either PR if you have any)

mnahkies added a commit that referenced this issue Oct 27, 2024
…functions

- previous names still exported as aliases for backwards compatibility,
  will probably keep these around forever, as sometimes they are more
  convenient

- only occurs when splitting the generated code by tag / slug / etc,
  when outputting a single file the original names are still used for
  now.

relates: #111
mnahkies added a commit that referenced this issue Oct 27, 2024
…functions

- previous names still exported as aliases for backwards compatibility,
  will probably keep these around forever, as sometimes they are more
  convenient

- only occurs when splitting the generated code by tag / slug / etc,
  when outputting a single file the original names are still used for
  now.

relates: #111
mnahkies added a commit that referenced this issue Oct 27, 2024
…tions (#258)

- previous names still exported as aliases for backwards compatibility,
will probably keep these around forever, as sometimes they are more
convenient, depends on how the consuming project is structured.

- only occurs when splitting the generated code by tag / slug / etc,
when outputting a single file the original names are still used for now.

- I'd like to further extend this to `ApiClient` exports for the client
SDKs, as this is normally where I find it most annoying

relates: #111
@mnahkies
Copy link
Owner

#258 / #256 have been released in https://github.com/mnahkies/openapi-code-generator/releases/tag/v0.15.0 which I believe mostly resolves this issue for the server template at least.

Edit: One other thing was having a global interface that uses the type/interface of the routes to ensure nothing is missed, as of right now, I could implement 90% of the route groups and forget to implement one of the route groups and nothing would error or complain at me to implement it, where as with a global interface if it wasn't implement it would complain.

I'm not totally sure what you're looking for here, a code example might be helpful, I have a guess but I'm struggling to see how it would be done usefully without coupling to the specific DI framework in this repo

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

No branches or pull requests

2 participants