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

Typescript support and in-memory generation #468

Open
underfisk opened this issue Dec 29, 2020 · 33 comments
Open

Typescript support and in-memory generation #468

underfisk opened this issue Dec 29, 2020 · 33 comments

Comments

@underfisk
Copy link

Reason/Context

Please try answering few of those questions

  • Why we need this improvement? There are a huge variety of projects using Typescript and this library offers no typings which is very necessary for those projects, for example, most of the module imports need to be via "require" instead of import
  • How will this change help? We can start using import without complaints, use Generator class with static typing and IDE help and create more libraries on top of async api
  • What is the motivation? Currently, i'm building a generator for Nestjs (currently there is only a package for Swagger) but for microservices, we need AsyncAPI, its being used. Nestjs is heavily used with Typescript and uses a lot of reflection to enhance the application and third-party libraries (such as swagger like i mentioned). Now regarding the in-memory generation that's something that swagger does, for the page and for its static assets instead of generating the files only. Generating the files will require OS permissions and in production that's very risky to give such, we should be able to generate to a memory buffer and simply serve from it, i think that would be beneficial instead of writing directly to FS. Also, i did nothing the template name is lazy loaded but its not cached which means that if we want to install the template as a module to generate faster we simply can't, we'll have to fetch each time our application bootstraps

Description

Please try answering few of those questions

  • What changes have to be introduced? Generating types (https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html) and provide a new method OR option that allow us to receive a buffer containing the static assets and the page build instead of directly write using fs module
  • Will this be a breaking change? No, it will just provide more enhancement
  • How could it be implemented/designed? Types is pretty much straightforward and in-memory generation could be discussed but i don't think it will interfere at all with the existing codebase
@underfisk underfisk added the enhancement New feature or request label Dec 29, 2020
@github-actions
Copy link
Contributor

Welcome to AsyncAPI. Thanks a lot for reporting your first issue.

Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@magicmatatjahu
Copy link
Member

@underfisk Hi! Thanks for an issue and for a comprehensive description.

As I see you would like to resolve this issue (btw. issue should be opened due to so that the NestJS community can find the issue more easily). It's very appreciate. If you need any help, feel free to ask here as well as on our slack.

Why we need this improvement? There are a huge variety of projects using Typescript and this library offers no typings which is very necessary for those projects, for example, most of the module imports need to be via "require" instead of import.

How will this change help? We can start using import without complaints, use Generator class with static typing and IDE help and create more libraries on top of async api

Exporting TS types shouldn't be hard to export, it's only one class :). Also as I remember TS should have possibility to import Generator class by import statement. only import should be ignored by @ts-ignore, but yeah, it's not a good DX.

What is the motivation? Currently, i'm building a generator for Nestjs (currently there is only a package for Swagger) but for microservices, we need AsyncAPI, its being used. Nestjs is heavily used with Typescript and uses a lot of reflection to enhance the application and third-party libraries (such as swagger like i mentioned). Now regarding the in-memory generation that's something that swagger does, for the page and for its static assets instead of generating the files only. Generating the files will require OS permissions and in production that's very risky to give such, we should be able to generate to a memory buffer and simply serve from it, i think that would be beneficial instead of writing directly to FS. Also, i did nothing the template name is lazy loaded but its not cached which means that if we want to install the template as a module to generate faster we simply can't, we'll have to fetch each time our application bootstraps.

As I understand you want to make similar package like @nestjs/swagger to provide code-first approach for documenting microservices written with @nestjs/microservices. Then after all operation with reflection (I mean provide and collect data from reflect-metadata by @Channel etc. decorators) you want to generate the page with documentation (like swagger-ui-express package do), and you probably choose our html-template. Am I right?

By in-memory generation do you mean writing result of generation to some variable in JS app as a string, yes? Maybe this is a stupid question, but it's only for clarify. In html-template, as I remember, we write everything to one file, so in-memory generation won't be a problem. However we have a lot of another templates which has a complex structure (not with one output file) so we should think about it when someone will write this feature. I won't tell you if there will be breaking-change, but they may appear.

About problem with fetching template on bootstrapping (probably you mean bootstrap app in NestJS), we have issue for that ->
#425

Workaround for your problem (at the moment) could be using @asyncapi/web-component. You can prepare docs from metadata and then host simple file, something like:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title>AsyncAPI Docs</title>
    <script src="https://unpkg.com/@asyncapi/[email protected]/lib/asyncapi-web-component.js" defer></script>
    <script>
        var schema = {{ YOUR_DOCS }};
        window.onload = function() {
            document.getElementById("asyncapi").schema=schema;
        } 
    </script>
    <style> 
    @import url("https://fonts.googleapis.com/css2?family=Roboto&display=swap");
    html,
    body {
    font-family: sans-serif;
    }
    .asyncapi__toggle-arrow:before {
        content: ">";
        font-size: 14px;
    }
    </style>
  </head>
  <body>
    <asyncapi-component id="asyncapi" cssImportPath="https://unpkg.com/@asyncapi/[email protected]/lib/styles/fiori.css"></asyncapi-component>
  </body>
</html>

I know that our react-component hasn't some features that has html-template like left navigation etc, but it should works. What do you think?

@underfisk
Copy link
Author

underfisk commented Dec 29, 2020

@magicmatatjahu Thanks for the fast reply and yes i want to turn the nestjs package available for everyone, i had a custom package already for my personal usage but it was very agnostic to the application i was doing but some points pinned there such as file system permissions are kinda critical.
It isn't hard to ignore and still use import but that's bad because it skips the purpose of typescript types (also i could create a type declaration module but its not practical and scalable) therefore i think if the generator export and create its own typings, that would be awesome
Yes you are right about the generating processing, that's pretty much what i had in mind (to keep this package very similar to the swagger codebase)

What i mean by in-memory generation is basically the output of the build instead of going to fs (as you said you write to a file), be stored in memory and not written in disk, that way we would have not to worry about permissions and stuff (and would be more secure). Also that probably would implicate using CDN for assets like tailwind css etc but that's pretty much what swagger does overall

The bootstrap issue, i have no problems running this, the issue is that generator will fetch remotely (installing all npm dependencies, downloading files etc etc) where it could cache and avoid doing that trip over and over. Why am i suggesting this? If you use hot-reload or fast refresh, you'll notice that after you change some line of code, your app will restart and boot all things again, and this is a heavy operation and caching it would make it way faster

Overall thanks for async-api, i had in mind a year ago of developing something close to Socket.IO because i've seen a lot of companies using socket-io but no documentation or playground (like swagger cURL) to test those but this specification is much better and is agnostic to the protocol, therefore it is excellent for microservices and Nestjs is a perfect fit to include Async API in its main "battery-included" packages

(Here is also the implementation of swagger UI for express, this can give you an idea of what i mean by in-memory generation https://github.com/scottie1984/swagger-ui-express/blob/master/index.js)

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Dec 29, 2020

(Here is also the implementation of swagger UI for express, this can give you an idea of what i mean by in-memory generation https://github.com/scottie1984/swagger-ui-express/blob/master/index.js)

Exactly, by this, I came up with this example using a react-web-component (in my previous comment) which is similar to the SwaggerUIBundle, so in YOUR_DOCS you can put your defined asyncapi schema.

As I said, types for Generator class won't be a problem, but we (team) have a free days to 4 January 2021, so after this day someone should take this task, or maybe if this is a urgent for you, if you want, you can create PR for that :) We will very appreciate. With in-memory generation there may be problems with complex templates as well as with generator's features like hooks, etc, so I can't tell you when it will be done.

About this bootstrap issue, I think that your problem is very similar to #425 I mean with this part with installing all npm dependencies over and over again in every changes, and as you wrote in first comment, installed template in app don't working, because the Generator re-installs the template anyway.

I forgot, but using Generator you can pass output option to render template as a string (not writing to file) https://github.com/asyncapi/generator/blob/master/lib/generator.js#L83 remember to pass also entrypoint https://github.com/asyncapi/generator/blob/master/lib/generator.js#L80 However it probably won't working for you (I didn't test it), because html-template need additional logic to put everything to one file (css, js etc) and output=string works only with one input template-file.

Overall thanks for async-api, i had in mind a year ago of developing something close to Socket.IO because i've seen a lot of companies using socket-io but no documentation or playground (like swagger cURL) to test those but this specification is much better and is agnostic to the protocol, therefore it is excellent for microservices and Nestjs is a perfect fit to include Async API in its main "battery-included" packages

Thanks very much on behalf of the entire team! I had this same situation (writing my own solution) with NestJS. I wanted to write something similar to Spring in NodeJS world, but then I found NestJS 😄

@underfisk
Copy link
Author

I will take a look at react-web-component but as you said it won't be similiar to the playground right? I need to check also the tradeoffs
Regarding types, i can wait, i have interfaces/classes that correspond on your API, it gave me some work but for now must be fine, the issue is always the same, i want to avoid having to maintain the types and check constantly the documentation which time i update, i think it make more sense for you guys

Exactly, by this, I came up with this example using a react-web-component (in my previous comment) which is similar to the SwaggerUIBundle, so in YOUR_DOCS you can put your defined asyncapi schema.

About in-memory-generation, if the 1st point solves the issue (which i'm not sure yet) it will not be so urgent but this is something that makes sense, as you know reading from disk is way expensive than from memory, i know complex templates can give some troubles (even from reading how the template engine is now working) but i'm pretty sure we can discuss the trade-offs. Btw from what i've seen in the source code, you write to a single file, i think creating a own data structure in-memory and having an fs proxy would not be that hard, i think the end result is the same, the difference is that instead of being built to a file, its in a memory buffer (as it would be by reading the file using fs, it. gives you a stream with bytes)

#425 yes the problem is very similiar, not sure yet if there will be an update on the package itself or if i have to add some workaround but it would make sense reading the cached modules

I forgot, but using Generator you can pass output option to render template as a string (not writing to file) https://github.com/asyncapi/generator/blob/master/lib/generator.js#L83 remember to pass also entrypoint https://github.com/asyncapi/generator/blob/master/lib/generator.js#L80 However it probably won't working for you (I didn't test it), because html-template need additional logic to put everything to one file (css, js etc) and output=string works only with one input template-file.

Yes i did read about it, i saw the ApI documentation and the source code, as you said it would gap on html-template so its not a viable option and i want this package to be extensive as Nestjs community packages are, they offer you the control and flexibility, therefore if some developer wants to pass his custom template, i have to provide that

Thanks very much on behalf of the entire team! I had this same situation (writing my own solution) with NestJS. I wanted to write something similar to Spring in NodeJS world, but then I found NestJS

I understand you because I have built from packages from Spring to nestjs such as Spring Cache, they have an excellent and mature layer and i did need it for our CQRS applications. Async API is very promising and i hope we can achieve great power using reflection to generate good documentation for microservices

@jonaslagoni
Copy link
Member

jonaslagoni commented Dec 29, 2020

As I said, types for Generator class won't be a problem

Just to mention, we have already implemented this and automated it in the parser. IMO we should probably follow the same setup if possible, I know I had some issues with jsdoc not generating the correct types right away, so we might need to do some tweekings.

@underfisk
Copy link
Author

@jonaslagoni But if you could generate that ahead of time and include in package json, NPM would include and typescript would detect (d.ts files) automatically

@magicmatatjahu Btw i forgot to suggest, what do you think about having "links" between the https://www.asyncapi.com/docs/specifications/2.0.0 (page) because i would like to add to jsdoc Message, Channel etc etc on the decorators, that way if anyone wants to open directly the specification it would make sense as a shortcut (Spring for example has this, which allows them to add references in code comments)

@jonaslagoni
Copy link
Member

@jonaslagoni But if you could generate that ahead of time and include in package json, NPM would include and typescript would detect (d.ts files) automatically

Yep, thats what I'm saying. We already have it working in the parser and should follow the same setup approach in the generator to enable types here 😄

@magicmatatjahu
Copy link
Member

@underfisk We generated types in AOT in parser (if we go with similar solution), so don't worry :)

Great idea about links :) Maybe I misunderstand your problem here, but links between sections work - you'll get url by clicking a given node in the right navigation (we don't have a anchors icon in headers).

@underfisk
Copy link
Author

@magicmatatjahu Makes sense generating it AOT xd but thanks
Ah ok i didn't notice that i thought it was like GitHub README where we had the anchor icon but now i see i can add reference, thanks again

@derberg
Copy link
Member

derberg commented Dec 30, 2020

Hey, few cents from my side:

  • I'm definitely with the idea to generate TS types the same way we already do it in the parser.
  • Regarding output and entrypoint options, see how they are used with html-template in the playground at the moment https://github.com/asyncapi/playground/blob/master/src/server/routes/html.js#L27-L28. In case this is not enough please suggest what changes you need exactly. Btw, html-template has a parameter that once set, inlines all files in a single HTML file

@underfisk
Copy link
Author

underfisk commented Dec 30, 2020

@derberg I have tested it and seems to just create a string, return it and if i pass (as you suggested) single file it will include, its working fine but since it lacks some types and in API it says Promise and not Promise when we do pass that options it seemed a bit confusing.
I think its better to create a new method that performs this, not sure what you guys think but also because this "in-memory generation" needs to avoid at all cost writing to the disk, its quite an expensive operation.
I also did notice that if i install the template as a module in my project and simply pass it (which i believe its not possible since the generator only supports the module name) would be way faster because we already have the dependency installed. What do you guys think of supporting passing the template module as a parameter? It would probably avoid the trip of fetching constantly

Regarding types, for generator, this is what i do have right now (not practical but its better than set to any type) but the library that i'm building i have 99% of the specification in Typescript, it might be useful to consider having it in a dedicated module instead of making it agnostic to Nestjs only

interface IGenerator {
  templateName: string
  targetDir: string
  entrypoint?: string
  noOverwriteGlobs: string[]
  disabledHooks: { [key: string]: string | boolean | string[] }
  output: 'string' | 'fs'
  forceWrite: boolean
  debug: boolean
  install: boolean
  // eslint-disable-next-line @typescript-eslint/ban-types
  templateConfig: object
  // eslint-disable-next-line @typescript-eslint/ban-types
  hooks: object
  templateParams: AsyncApiTemplateOptions
  generate: (document: any) => Promise<void>
  generateFromURL: (url: string) => Promise<void>
  generateFromFile: (path: string) => Promise<void>
  generateFromString: (yaml: string) => Promise<void>
}

@derberg
Copy link
Member

derberg commented Dec 30, 2020

@underfisk the problem with reusing template as dependency is known but not yet solved #425 so at the moment just do what playground does, which means, first-run installs template and next ones reuse what is installed on the server.

the challenge with the idea to split render-to-string to a separate function is that then we would have too many functions in my opinion, generate and generateToString, then generateFromURL and generateFromURLToString, etc. I guess we just need to make sure we have well described and generated types.

@underfisk
Copy link
Author

@derberg I agree with you with the current API, there are a lot of methods, in fact with static typing/generating declarations, the problem would be solved and probably the github api documentation would not be so necessary at all, that's a great advantage of using typescript
I understand there is a problem with #425 but what you guys do behind the scenes is fetch with the module name, what if we could eagerly send you the module and you just use it (since it is already installed), that way you would avoid a trip, i do understand it might become a challenge and that one might be a breaking change (or not). I have already set what the playground does have, seems okay for now, and would do the trick (at least my m1 is not complaining xd and boot is fast)

@derberg
Copy link
Member

derberg commented Dec 30, 2020

I understand there is a problem with #425 but what you guys do behind the scenes is fetch with the module name, what if we could eagerly send you the module and you just use it (since it is already installed), that way you would avoid a trip, i do understand it might become a challenge and that one might be a breaking change (or not)

once you point to the local template (for example how we do when developing templates) we do not perform fetching but create a symlink in node-modules of the generator that points to the local template. I think it would be great if we could support a scenario that you add template do dependencies and we just reuse what you already have in node-modules.

@underfisk
Copy link
Author

@derberg In fact, even so if we pass the module you already have what you need, if you symlink or not, the content is passed to you already, that way you will prevent the constant "fetching". Well, in my case it did not run only 1 think the npm install of puppeteer etc, it did all the time the app was bootstrapping or each time a test suite was ran
Also the documentation regarding the generator kinda "misleads" at output option, because if you pass output as string then you shouldn't need to pass a directory, ofc moving the directory out from the 2nd parameter to the options object would provoke a breaking change, therefore if we could have a note that even a blank target directory would do just fine (that it would be ignored) would probably help to understand a bit of what's going to be done
Right now then the only issue i see is the #425 and types, 425 relating to the in-memory generation but my library will still do some caching to avoid building template each time app bootstraps with hot-reload
Thanks again for replying to this topic, i'm glad this community is supportive 💯

@derberg
Copy link
Member

derberg commented Dec 30, 2020

We will help more in 2021, we need to load batteries now 😄

@underfisk
Copy link
Author

Indeed :D

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2021

This issue has been automatically marked as stale because it has not had recent activity 😴
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 ❤️

@github-actions github-actions bot added the stale label Mar 1, 2021
@derberg derberg removed enhancement New feature or request stale labels Mar 1, 2021
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2021

This issue has been automatically marked as stale because it has not had recent activity 😴
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 ❤️

@github-actions github-actions bot added the stale label May 1, 2021
@derberg derberg removed the stale label May 4, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2021

This issue has been automatically marked as stale because it has not had recent activity 😴
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 ❤️

@github-actions github-actions bot added the stale label Jul 4, 2021
@derberg derberg removed the stale label Jul 5, 2021
@derberg
Copy link
Member

derberg commented Jul 5, 2021

@underfisk hey, is this something you could help implementing?

@underfisk
Copy link
Author

@derberg I think so, i've seen some nestjs wrappers that they were using OpenAPI codegen approach to generate for async api, in my opinion we could generate the html and cache it and leverage from services like memcache if the user wants to.
@derberg What's your opinion in just generating the html in memory and send the "text" as html SSR from the express/fastify adapter route that we generate? (This is probably more directed to Nestjs but overall i see people that would leverage on having this on express vanilla or not)

@derberg
Copy link
Member

derberg commented Jul 5, 2021

for sure first of all for me important is that we do not introduce 2 things at the same time, TS + in-memory generation.

some things changed since the time you created this issue. HTML template is totally different now. It is fully reusing https://github.com/asyncapi/asyncapi-react/tree/next. The react component is also now better optimized. Maybe you do not really need generator for NestJS plugin? https://github.com/asyncapi/html-template/blob/master/template/index.html

@underfisk
Copy link
Author

@derberg I think i will still need to check this HTML, i might not need and i might be able to use that template is just that in Nestjs you rely on decorators and the metadata they provide, my own nestjs-asyncapi module scans for the routes that have that same metadata and generate all the YML but if the template is capable of being sent from the server then i'm fine with it :)

@derberg
Copy link
Member

derberg commented Jul 5, 2021

unfortunately, my knowledge about Nestjs is close to 0 😞 except knowing the creator is from the area where I live 😄

my point is that if you can do such things with react component directly https://github.com/asyncapi/asyncapi-react/blob/next/docs/usage/nextjs.md#usage-with-ssrssg-static-documentation then it just sounds to me that using generator with html-template (that actually uses react component) is a kind of unnecessary overhead. But yeah, I base my opinion not knowing anything about NestJS and it's capabilities.

@magicmatatjahu I think you played with Nestjs some time ago

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Jul 5, 2021

Yes, I played :)

Ok, so as I remember the problem, you don't want to generate the template to files each time if someone will change something in code (by hot reloading), because generating takes a few extra seconds, so you want to generate the docs as SSR page, so generating will be "fired" only when someone go to the docs path - it will reduce the overhead with generation's time and FS permissions. Am I right?

If yes, you can use the "newest" code for html-template, but you have to transform the handlebars code to the corresponding JS template literals, so you see how we generate the React component in server (here is a Nunjucks filter) and then hydrate the component in browser - https://github.com/asyncapi/html-template/blob/master/template/index.html#L19 - if you will have problem I can attach some code snippet how it could look :)

By this you can make own "in-memory" generation, but only for the html-template.

Scan and reading the metadata from corresponding methods in Nest is a separate problem and I don't see correlations here, because template is the last part of the module. Also be aware that NestJS has possibility to define that given channel/method has request/reply pattern (I don't remember which metadata adds it - by @Message or @Event decorator) and at the moment in AsyncAPI we have only issue for that - asyncapi/spec#94

If you have any questions, feel free to ask :)

@underfisk
Copy link
Author

underfisk commented Jul 5, 2021

@magicmatatjahu Yes you're right, i dont need AOT on the template generation, its a simple JIT when the user hits the docs route and i just need to seed the in-memory scanned YML and in nestjs i can keep in service the generated metadata so i avoid re-scanning each time the user hits the route
My concern was about the template but i see that its possible, once again thanks for the insight

@magicmatatjahu
Copy link
Member

Exactly, but template itself hasn't that possibility, but you can make full SSR based on template :)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2021

This issue has been automatically marked as stale because it has not had recent activity 😴
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 ❤️

@github-actions github-actions bot added the stale label Sep 4, 2021
@derberg derberg removed the stale label Sep 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

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 ❤️

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity 😴

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 ❤️

@github-actions github-actions bot added the stale label May 12, 2022
@derberg derberg removed the stale label May 16, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity 😴

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 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants