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

Avoid code repetition in templates filters #212

Closed
derberg opened this issue Feb 6, 2020 · 8 comments · Fixed by #298
Closed

Avoid code repetition in templates filters #212

derberg opened this issue Feb 6, 2020 · 8 comments · Fixed by #298
Assignees
Labels
enhancement New feature or request released

Comments

@derberg
Copy link
Member

derberg commented Feb 6, 2020

Description:

Templating in the generator is based on Nunjucks. One of its features is filters that allow you to apply custom functions on variables https://mozilla.github.io/nunjucks/templating.html#filters.

Generic filters/functions should be abstracted to one common location and shared with other filters. There is no specific proposal on how to do it, it can be just a part of the generator but in one common place, specific filters should stay in current locations.

We also need to take into account users that do not use existing templates located in repository but instead, they keep them privately. We need to have a good solution here, that will allow these users to easily import generic filters to their templates if they want, so they can clean up their filters.

Reason:

The current issue is that those filters are not shared but every template have own filters, even the basic ones.

Example: a log function is duplicated here https://github.com/asyncapi/generator/blob/master/templates/html/.filters/all.js#L116 and https://github.com/asyncapi/generator/blob/master/templates/markdown/.filters/all.js#L3.

@derberg derberg added the enhancement New feature or request label Feb 6, 2020
@derberg
Copy link
Member Author

derberg commented Feb 7, 2020

I just noticed that we have the same situation with .hooks, they are also duplicated across templates, like for example in nodejs and nodejs-ws

@fmvilas
Copy link
Member

fmvilas commented Feb 7, 2020

That's a great catch. I think we should probably attack #214 first. This way we could implement hooks as packages and filters as packages. Otherwise, we'll bloat the generator. What do you think?

@derberg
Copy link
Member Author

derberg commented Apr 10, 2020

At the moment filters registration works this way that https://github.com/asyncapi/generator/blob/master/lib/generator.js#L331 walks over all files from filters dir. We still should support custom template filters provided for specific template only.

There is basically no other option for generic filters than putting them in one library and publish to npm as @asyncapi/generator-filters. Then you would add such a library to dev dependencies of your template.
Now, how to let the generator know that it needs to register those generic filters? I suggest another parameter in .tp-config.json like additionalFilters or registerFilters or just filters 😄 where you would provide a package name that generator should walk over.

Open questions:

  • should we keep a clear separation between filters and hooks and put them in separate packages, and also have config for registerHooks or should we put them in one @asyncapi/generator-helpers package, but still specify separately in config. I don't have a strong opinion here, clear separation sounds like better approach, but dunno for 100%.
  • do you see a way of getting rid of this "manual" step where you have to add filters explicitly to the config? I don't and I think it is ok, but maybe you have some good idea here
  • in my opinion, filters registration, both remote (the ones that are in the package on npm) and the custom local ones should be unified. I mean that local ones should also be added through the config file, as local path reference, we can keep filters dir as the default of course.

Any thoughts?

@Tenischev
Copy link
Member

Hi, i'm from Java world and since Java 9 we split everything onto small packages :) which are depended/reused by other packages picture. Another example could be Apache Commons where they split libraries by purposes.
About hooks and filter - i prefer the current situation when they are placed separately.

@fmvilas
Copy link
Member

fmvilas commented Apr 10, 2020

should we keep a clear separation between filters and hooks and put them in separate packages, and also have config for registerHooks or should we put them in one @asyncapi/generator-helpers package, but still specify separately in config. I don't have a strong opinion here, clear separation sounds like better approach, but dunno for 100%.

I think they should be separated. Even each hook and each filter should be a separate package (not necessarily a separate repo). If we see useful groups of filters or hooks, we can maintain another package that would be a sum of them, like stdlib in C or Babel presets.

@derberg
Copy link
Member Author

derberg commented Apr 17, 2020

Ok, filters only

initial PR ready. There is still some cleanup I have to do, but the idea I had in mind is in place.

What done:

What left:

Have a look before I jump to rework rest of the templates ;)

@derberg
Copy link
Member Author

derberg commented Apr 28, 2020

@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 0.41.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants