Generator: Avoid hooks repetition similar to what we did with filters

[derberg]

Reason/Context

We duplicate code across templates and we should have a central library with reusable hooks

Description

Check out how we did it with filters and we probably should do the same to hooks https://github.com/asyncapi/generator/issues/212

[Tenischev]

Reusable hooks are OK, but the question for me: How we should define which hook from shared library should be applied in template and which not?
With filters it was easier, because they just functions and you should call them in the code to apply. When hooks are called just by the the presence in .hooks folder.

[derberg]

@Tenischev how about doing it in the config? with filters we do:

"filters": [ "@asyncapi/generator-filters"]

maybe with hooks we would do:

"hooks": [ 
  ["@asyncapi/generator-hooks", [ "thisHookTriggersFirst", "thisHookIsNext, "ThisHookIsLast"]]
]

What do you think?

[Tenischev]

Yes, good approach, will work.
It will be pretty cool if we could guarantee order of execution.

And as following idea - provide some mechanism to define an order of execution for custom hooks from .hooks folder.

[derberg]

@Tenischev exactly, this is why in my proposal you have an array, order in the array would indicate the execution order.

Regarding local, I'm wondering if it should be a custom mechanism or you maybe should just add local hooks also manually to "hooks" in case you want to manipulate the order 🤔

[Tenischev]

I think the same mechanism is better.
Something like:

"hooks": [ 
  "@asyncapi/generator-hooks": [ "thisHookTriggersFirst", "thisHookIsNext, "ThisHookIsLast"],
  "local": [ "myCustomHookFirst", "myCustomHookSecond" ]
]

But actually another question. Now we have generate:before and generate:after. What will be if i put hooks in such order?

"hooks": [ 
  "@asyncapi/generator-hooks": [ "thisHookWithTypeAfter", "thisHookWithTypeBefore", "thisHookWithTypeAfter"]
]

[derberg]

@Tenischev order will be respected only for specific types of hook, right?

[Tenischev]

Sure, but we should make it clear from the docs. Just to be aware of it.

[derberg]

I think I'm gonna descope order of execution from this issue to not copmlicate things to much.
I'm not fully into idea of handling order through config but rather the hook itself, where we could discuss and long term introduce weights on a hook level. The generator would simply sort the hooks of different types basing on the weight. Just a thought, but not for this one imho

[derberg]

I’m pretty much done with the concept, have ready implementation and just need to refactor it, modify tests a bit and make it super pretty.

new config hooks property looks like

  "hooks": {
    "@asyncapi/generator-hooks": "createAsyncapiFile"
  }

The outcome is the change in the meaning of hook name, because we have now different hooks, generate: after and generate:before, and to configure hooks we will now have to provide names for hook functions. hook name will be createAsyncapiFile and hook type will be generate: after.

This change affects the current disabledHooks. In my opinion, we must be able to not only disable the whole hook type but also a particular hook by its name.

Questions:

• first of all what you think about above
• and would you like me to do it now, or rather in a follow-up ticket? it will probably double-up code changes which can make PR review much complicated

[Tenischev]

I vote for follow-up ticket.
Could we somehow make hook name optional? To avoid breaking changes for templates and allow them smoothly migrate?

[derberg]

@Tenischev actually hook name is not mandatory, but the way you expose hooks changed so breaking change is inevitable. No worries though, like in case of filters, I will create PR's in templates and not merge until PRs in all templates are ready

[derberg]

Main PR for review - https://github.com/asyncapi/generator/pull/334
Generator-hooks lib - https://github.com/asyncapi/generator-hooks/pull/1
Nodejs template - https://github.com/asyncapi/nodejs-template/pull/13

Missing PRs that will be added once above is approved:

• https://github.com/asyncapi/nodejs-ws-template
• https://github.com/asyncapi/java-spring-template
• https://github.com/asyncapi/java-spring-cloud-stream-template

[derberg]

All released, closing