event-catalog / generator-asyncapi

AsyncAPI generator for EventCatalog
https://www.eventcatalog.dev/docs/development/integrations/async-api/intro
Other
9 stars 4 forks source link

Better handling of service filename #42

Closed hpatoio closed 1 month ago

hpatoio commented 1 month ago

Use Case

I run the command npm run generate with an AsyncAPI file whose info.title was:

info:
  title: Foo Service. See http://foo.com
  version: 1.0.0

Since generator uses title as filename this was created in services folder

├── Foo Service. See http:
│   └── foo.com
│       ├── index.md
│       └── inventory-service.yml

Proposed Solution

We can have different solution:

  1. We "sanitize" the value in info.title before use it as a filename.
  2. We allow an extra property x-eventcatalog-service-id that allow to specify a machine-friendly name for the service. If this is not set we can still fallback to point 1.

Something like:

info:
  title: Foo Service. See http://foo.com
  version: 1.0.0
  x-eventcatalog-service: foo-service

Implementation Notes

No response

Community Notes

boyney123 commented 1 month ago

Thanks for raising @hpatoio .

I released a new version of the generator @eventcatalog/generator-asyncapi@1.0.4 with support for a new folderName property. Let me know if this helps.

https://www.eventcatalog.dev/docs/development/plugins/async-api/api#required-fields

hpatoio commented 1 month ago

Everything works. I dug a bit into the code and not it's more clear to me how things works.

IMHO you should make id mandatory. The code, and the conf, would be much simpler/clear (no fallback). In this way you could also remove folderName and use id.

My 2 cents.

boyney123 commented 1 month ago

Everything works. I dug a bit into the code and not it's more clear to me how things works.

IMHO you should make id mandatory. The code, and the conf, would be much simpler/clear (no fallback). In this way you could also remove folderName and use id.

My 2 cents.

I actually quite like this tbh, keeping things simple. We have a breaking change incoming @glc-omid #40, wonder if it makes sense to add this. I think it does... will make a PR this week to attach to the next major version.

glc-omid commented 1 month ago

We can also une slugified file name if serviceid not present

hpatoio commented 1 month ago

We can also une slugified file name if serviceid not present

I think that this just make things more complicated without any real value to the user. It's also more work on documentation side.

I don't get what's the problem with a conf like

services: [
  { path: path.join(__dirname, 'asyncapi-files', 'inventory-service.yml'), id: 'my-service'}
],
  1. You know exactly what will be the id of your service
  2. You can use any "format" you want. CamelCase, snake_case etc ...
  3. If for any reason you need to change the filename don't have to worry about any possible side effect

Personally I would also remove the possibility to specify the folder for the specs. So we might have a conf that looks like

services: [
  { specs: 'inventory-service.yml', id: 'my-service'}
],
boyney123 commented 1 month ago

Thanks @hpatoio , this has now been implemented in 2.0.0. id is required, foldername is gone! Thanks for the idea, I think its a good one! Docs updated.