feat: initial try to get the parser working #1026
Conversation
Shurtu-gal
requested review from
magicmatatjahu,
KhudaDad414,
Amzani and
asyncapi-bot-eve
as code owners
|
✅ Deploy Preview for asyncapi-studio-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for modest-rosalind-098b67 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for studio-next failed.
|
|
|
@Shurtu-gal Hey man, I want to be a part of this initiative and provide some help if I can. For that, I tried to migrate the studio to Next.js before and faced some challenges but overall it was smooth. But I had to have Can you provide what the challenge is here? And how you are trying to resolve it? 🙏 |
|
@KhudaDad414 this is the error which I have been facing for the last couple of days. |
|
@Shurtu-gal the error seems... strange to say the least. it works great in dev and builds fine. There may be some rough edges that we need to iron out. |
@KhudaDad414 then is there any benefit in shifting to Next.js then 🤔 |
|
TBH, I don't think there is any for the current studio. We can structure it a little better to have the "use client" in Editor's own component. Why? Three main components in the page are the editor, navigator, and the preview window. The editor can't be server side rendered, based on what I could gather. (Code mirror and Monaco use react hooks in their codebase. They both were made to be rendered on the client) The other two other main components rely on the editor to display their info, so they have to be client side rendered as well. What about when we have the visual editor? I believe it can be server side rendered. Plus, we can do what My point is that there isn't any Next benefit now, but there can be in the future. |
|
Agree with the API philosophy, maybe can make an API route which does the requisite parsing and populates the errors, and other stuff in |
|
@Shurtu-gal the thing is that we can't keep the AsyncAPI document on the server. and if it has to be on the client, then the current approach (keeping the file in the browser and using state management) is the only approach. |
|
I believe there was a misunderstanding. It will be like However, while writing this I can think of the disadvantages as well, as there will be too many calls to the API functions. |
|
@Shurtu-gal, @KhudaDad414 there are no immediate benefits, but this will unlock a couple of features:
If we have an alternative to |
|
So should I move forward with using 'use client' everywhere? But I believe this will cause problems down the line 😔. Maybe 🤔 we could resolve this, let me debug it a little Also @KhudaDad414 @Amzani your help and inputs would be great too. |
|
@Shurtu-gal I am afraid there is no other way. |
|
@Shurtu-gal @KhudaDad414 what if we use |
That would work, let me try it out and I will let you know. |
|
@Shurtu-gal @KhudaDad414 NextJS might sound useless in the beginning, especially as the components are tightly coupled to Browser API, for instance, the TopBar might directly use browser Local storage, so we can't make it a Server Side component. Once we complete the migration we can follow this pattern that @fmvilas defined in the following: https://github.com/asyncapi/studio/pull/739/files (this is still relevant), so a component like TopBar could be a server-side component for instance. |
I tried that, but it is showing the same errors. Furthermore @KhudaDad414 your implementation is also not working. FYI there was a similar try in this direction before too in this PR. |
|
@Shurtu-gal let me give it a try. 🤔 |
|
Hey @Shurtu-gal, I have opened a PR with a solution. (there were too much complications pushing the changes to this PR) |
|
@KhudaDad414 I am fine with it, it's a great solution, however, we still need to think of something so that we don't need an app-level Closing this PR then. |
Description
Related issue(s)
#684