generator
generator copied to clipboard
React templates make generation process longer
Describe the bug
✓ generated using Nunjucks template (492ms) #html-template
✓ generate using React template (4875ms) #markdown-template
Markdown template is the simples one we have, yet it takes 5s to generate asyncapi.md with it. Which is really a lot and I bet it will be worse with more complicated templates.
How to Reproduce
Just try to generate with HTML template and then markdown one and you'll see that there is a time difference each time.
The problem is in transpilation process with babel. @magicmatatjahu checked and:
5 times in a row with transpilation in each render:
3.768s
2.391s
2.236s
2.260s
2.414s
Average: ~2.6s
And here is a time without transpilation in each render:
191.515ms
194.49ms
185.167ms
192.192ms
199.48ms
Average: ~0.193s
Expected behaviour
IMHO by default, when you run generation, an already transpiled template is used to speed up (yes, it means we publish transpiled files to npm too). The generator is smart and spots transpiled files are missing and it needs to run transpilation. Users (template developers) also have a flag that allows them to explicitly run transpilation during development
I see two options to have transpiled template in npm:
--compileflag in our cli - it probably the easiest way to resolve problem and it provides a good DX for template's developer, but I don't know if new flag should work only with React.- instruct people how to write simple script in js to run transpilation in
prepublishscript inpackage.json- suggested by @jonaslagoni
--compile flag in our cli - it probably the easiest way to resolve problem and it provides a good DX for template's developer, but I don't know if new flag should work only with React.
I think it is ok that --compile is just for react as anyway this is a flag that is related purely to development only
instruct people how to write simple script in js to run transpilation in prepublish script in package.json.
I think this is fine, we just need to make it clear in docs and present it in the template for templates
Let's wait for other answers, what they think about it. After that, I would like to implement the solution.
@derberg Maybe we should transfer this issue to generator repo? I mean, in this repo shouldn't be changed anything, issue is only related with generator.
@magicmatatjahu good point, done
This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:
This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:
This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
@magicmatatjahu I don't remember what compile flag was suppose to do
@derberg To compile the React template and use that "compiled" templated from each run. At the moment we transpile/compile React component by babel on each generation and it is a waste of time.
@magicmatatjahu thanks, now I remember. I just think now that it should work like this by default. When you run generation, generator first look for sources of compiled template, if provided, and if not, performs old way. And yeah still in template for templates we need to show how to get it into npm
I just think now that it should work like this by default. When you run generation, generator first look for sources of compiled template, if provided, and if not, performs old way
Exactly in this way, look for transpiled files, if don't exist then run transpilation/compilation and generate template.
I am getting a bit tired of the long transpile process 😅
@magicmatatjahu would you be able to formulate this into a good first issue? 🙂
@magicmatatjahu I second what @jonaslagoni wrote because of this thread https://asyncapi.slack.com/archives/CQVJXFNQL/p1648304288930959
we are getting more and more React-based templates, means more unhappy users.
btw, I was wondering if we could not make it
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
Reopening. Now with budget in bounty program maybe we can find someone who could work on improving things
What needs to be done:
- In Generator we need to:
- enable skipping default transpilation. In other words, we should actually make transpilation not be triggered. Related code https://github.com/asyncapi/generator/blob/master/lib/renderer/react.js#L19
- to enable recompilation, we should enable option
compile(to be added in https://github.com/asyncapi/generator/blob/master/lib/generator.js) that by default is set tofalse. When set totrue, transpilation runs again
First bullet point might be considered a breaking change, which is not a problem as we can release it as breaking together with https://github.com/asyncapi/generator/pull/925 and also to use this feature, template developers will anyway have to make changes in their projects to enable release of transpiled files - so they can also change template settings so new version of their template supports latest generator
-
In CLI new flag will need to be enabled
-
In
html-templatewe need an example flow on how to enable this new feature to speed up development but also DX in case of generation of output -
Educate template developers: have a document that describes how stuff work and make sure in all templates under AsyncAPI org there are issues to set things up
1-3 can be pushed through bounty program 2-4 can be pushed as good first issues as after 1 and 3, 2 and 4 will be trivial
@jonaslagoni @magicmatatjahu sounds like a plan? also we could use this as onboarding of potential new maintainer in generator
@derberg makes sense to me 👌
A non-asked opinion here (not owner): I like the suggested solution 👍
actually I think number 1 is not needed
I opened a bounty issue for number 3 https://github.com/asyncapi/html-template/issues/558. I noticed that only first generation takes time, and then once transpilation files are there, it is fast, so looks like babel that we use underneath is pretty smart. So lets first modify release for html-template and see.
Hey @derberg as asyncapi/html-template#558 is near to completion can i open number 3 issue in the CLI
Ok, so https://github.com/asyncapi/html-template/issues/558 is completed and __transpiled folder is now part of package, but yeah, we still need to do modifications in generator as logs still show:
[BABEL] Note: The code generator has deoptimised the styling of /xxx/.nvm/versions/node/v16.13.0/lib/node_modules/@asyncapi/cli/node_modules/@asyncapi/generator/node_modules/@asyncapi/html-template/template/js/asyncapi-ui.min.js as it exceeds the max of 500KB.
which means transpilation runs locally anyway
of course it runs faster as some files are there, as they are now included in a package - but we should anyway try to avoid triggering transpilation if not needed
//these are times for template v2.2 without transpiled files
real 1m17.492s
user 0m37.739s
sys 0m24.042s
//these are times after transpilation is included in the package in v2.3
real 0m19.055s
user 0m25.391s
sys 0m3.009s
so we gained at speed significantly anyway! thanks @utnim2
is near to completion can i open number 3 issue in the CLI
@utnim2 CLI is point 2 and we need point 1 first. Should be an easy PR 🤔 you think you can work on it?
@utnim2 CLI is point
2and we need point1first. Should be an easy PR 🤔 you think you can work on it?
is point 1 needed you have said here that point 1 is not needed
yeah, so we see optimization worked but mainly because of babel that is smart and just tries to regenerate what is not available. But logs still show babel is triggered, so yeah, we should skip it entirely if not needed. So generator should perform transpilation only if explicitly "asked" or if transpiled folder is not present
@derberg will you make the issue or should I proceed with a PR directly
@utnim2 no need for separate issue, just refer to this one
@utnim2 CLI is point
2and we need point1first. Should be an easy PR 🤔 you think you can work on it?
Done in this #1177 cc @derberg