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

Can playground be removed? #752

Closed
jonaslagoni opened this issue Sep 4, 2023 · 23 comments
Closed

Can playground be removed? #752

jonaslagoni opened this issue Sep 4, 2023 · 23 comments
Labels

Comments

@jonaslagoni
Copy link
Member

Description
I have a hard time figuring out why we have the playground in this library.

Can it be removed or is there reason to keep it? 🤔

@derberg
Copy link
Member

derberg commented Sep 4, 2023

it is there for local development purposes, so you can quickly check the component in action

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Sep 4, 2023

Do you ever check a single component or is it just to see the full rendering?

Copy link
Member

derberg commented Sep 4, 2023

it is for checking full rendering on local, this is why with each new feature added we add it also the a "test" asyncapi file we have there.
Basically for much easier and faster review of how something looks in UI

@jonaslagoni
Copy link
Member Author

Is that not what the e2e tests are for?

I find it really strange to have an app inside a library ONLY for testing, when the same thing is done in the e2e tests 😄

@derberg
Copy link
Member

derberg commented Sep 4, 2023

it is not the same.

automated testing is a different thing than going to see an actual playground, seeing the component in action:

  • any user of react can see how stuff will be rendered (not possible with tests)
  • any reviewer can quickly see how UI is implemented (not possible with tests).
  • anyone who wants to integrate the component in some app, like playground, can see a real example how it is done (not possible with tests)

when you develop https://github.com/asyncapi/EDAVisualiser, you only write tests? don't you manually check how your new stuff behaves?

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Sep 4, 2023

I am not saying that you should not see it in effect.

But you can do the same with the e2e tests, i.e. just open the HTML document in the browser and it renders it fine.

image

Hence my question, cause to me, right now it's an unnecessary project to have included in this repo 🙂

@derberg
Copy link
Member

derberg commented Sep 4, 2023

test do not show library used in action, if it is still usable after a PR

what is the point of having examples in https://github.com/asyncapi/EDAVisualiser and pushing them to GH pages?

@jonaslagoni
Copy link
Member Author

test do not show library used in action, if it is still usable after a PR

No, but if you manually have to clone PR, go to playground, build everything and run it, and then see it in action. Then you might as well do clone PR, build the library, go to html documents and open. And then remove an entire app from the repo that you no longer have to maintain 😄

what is the point of having examples in https://github.com/asyncapi/EDAVisualiser and pushing them to GH pages?

I am only pushing the react example, the rest are for enabling users to see how to integrate with the library in different cases.

Thats not what the playground is about?

Ahhhhh, you deploy it here: https://asyncapi.github.io/asyncapi-react/

Why didnt you say that, that is what it's used for and not just local development 😆

@derberg
Copy link
Member

derberg commented Sep 4, 2023

what? 😄 it is 👇🏼 and in readme 😛

Screenshot 2023-09-04 at 16 16 03

I also wrote

anyone who wants to integrate the component in some app, like playground, can see a real example how it is done (not possible with tests)

Monday mood kicking in big time huh? 😆

@jonaslagoni
Copy link
Member Author

what? 😄 it is 👇🏼 and in readme 😛

It's not like I always look there unless there is a specific reason, and you did not say it either that it was deployed on each commit 😆

@fmvilas fmvilas reopened this Sep 5, 2023
@fmvilas
Copy link
Member

fmvilas commented Sep 5, 2023

I'd like to reopen this issue. It's not true that we need playground to try the component. We just need a dummy React app that would let us feed the React component with any AsyncAPI file we decide during development. The code editor, the configuration tab, and definitely https://asyncapi.github.io/asyncapi-react/ doesn't need to exist.

If someone wants to use the React component, that's what Studio is for. If we want to showcase it with different examples, a simple Examples page like Jonas did with EDAVisualiser should be more than enough IMHO.

Copy link
Member

derberg commented Sep 6, 2023

sure, but just curious, what is the point of removing something that is already there and working?

If someone wants to use the React component, that's what Studio is for

no, it is not. Studio is just one of consumers, showing complex next.js use case for the component in a complex app. In HTML template we show how it can be used as website generated in CI. Playground is super simple client side editor, perfect example. If you look into

Actually we had few requests in the past to have playground pushed to npm:

@fmvilas
Copy link
Member

fmvilas commented Sep 6, 2023

what is the point of removing something that is already there and working?

Maintenance burden (especially the deployment issues that may appear, as Jonas experienced recently). Also, to remove confusion.

no, it is not. Studio is just one of consumers, showing complex next.js use case for the component in a complex app. In HTML template we show how it can be used as website generated in CI. Playground is super simple client side editor, perfect example.

I'm sorry but Studio is currently not more than just this editor. How is this not the same? Also, in the plans for Studio, there isn't an intention to remove the "code editor" view. Studio is deployed and ready to use and we can make its "editor view" a separate component and push it to npm if needed but don't know why you would want to have the playground in the React component codebase.

#698

In the #698 issue, the person is actually looking to include Studio editor, not the one here. They just thought the playground here is the editor in Studio, and that's precisely the confusion I want to avoid.

Swagger does it, and people use it, which makes sense

Absolutely, let's do it but let's push the one that we're maintaining on Studio instead.

@derberg
Copy link
Member

derberg commented Sep 7, 2023

Maintenance burden (especially the deployment issues that may appear, as Jonas experienced recently)

What maintainance burden? There are only bumps needed. And the recent change from Jonas us just few lines of code.

Screenshot 2023-09-07 at 16 53 27

Also, to remove confusion.

what confusion do you mean? readme explains what it is.

I'm sorry but Studio is currently not more than just this editor

but plans are different, so we should not only think about current state.

and it is not just an editor, it has many additional features like for example code generation. It is a complex editor, when compared to playground.

we can make its "editor view" a separate component

would be nice, would address some of the needs from users

why you would want to have the playground in the React component codebase.

It is not part of code base, separate project in monorepo.
And because for now Studio is complex, based on next.js and is not a nice easy example on how react component could be used


When we have react component split into multiple ones with nice components gallery that users can go through + studio having editor as component - then yeah, let's remove playground

But now, seeing it requires almost 0 work, I don't see the point.

@derberg
Copy link
Member

derberg commented Sep 13, 2023

There is some more serious issue with the Playground. I solved all compilation related issues in #767 but there are runtime issues.

I managed to narrow down that it stopped working with: #746

Uncaught Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.
    at resolveDispatcher (react.development.js:1280:1)
    at useState (react.development.js:1306:1)
    at AsyncApiLayout (Layout.js:16:1)
    at renderWithHooks (react-dom.development.js:12872:1)
    at mountIndeterminateComponent (react-dom.development.js:15202:1)
    at beginWork (react-dom.development.js:16139:1)
    at HTMLUnknownElement.callCallback (react-dom.development.js:169:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:213:1)
    at invokeGuardedCallback (react-dom.development.js:261:1)
    at beginWork$1 (react-dom.development.js:20042:1)
    at performUnitOfWork (react-dom.development.js:19163:1)
    at workLoopSync (react-dom.development.js:19142:1)
    at performSyncWorkOnRoot (react-dom.development.js:18812:1)
    at react-dom.development.js:9783:1
    at unstable_runWithPriority (scheduler.development.js:572:1)
    at runWithPriority$1 (react-dom.development.js:9738:1)
    at flushSyncCallbackQueueImpl (react-dom.development.js:9779:1)
    at flushSyncCallbackQueue (react-dom.development.js:9769:1)
    at scheduleUpdateOnFiber (react-dom.development.js:18329:1)
    at Object.enqueueSetState (react-dom.development.js:11086:1)
    at push.../library/node_modules/react/cjs/react.development.js.Component.setState (react.development.js:408:1)
    at AsyncApiComponent.<anonymous> (AsyncApi.js:227:1)
    at step (AsyncApi.js:122:1)
    at Object.next (AsyncApi.js:71:1)
    at fulfilled (AsyncApi.js:30:1)
index.js:1191 The above error occurred in the <AsyncApiLayout> component:
    in AsyncApiLayout
    in AsyncApiComponent
    in AsyncApiComponent (at Playground.tsx:92)
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div (at Playground.tsx:91)
    in div (created by SplitWrapper)
    in SplitWrapper (at SplitWrapper.tsx:6)
    in SplitWrapper (at Playground.tsx:63)
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div (at Playground.tsx:61)
    in Playground (at src/index.tsx:10)

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries.
console.<computed> @ index.js:1191
AsyncApi.js:39 Uncaught (in promise) Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.
    at resolveDispatcher (react.development.js:1280:1)
    at useState (react.development.js:1306:1)
    at AsyncApiLayout (Layout.js:16:1)
    at renderWithHooks (react-dom.development.js:12872:1)
    at mountIndeterminateComponent (react-dom.development.js:15202:1)
    at beginWork (react-dom.development.js:16139:1)
    at HTMLUnknownElement.callCallback (react-dom.development.js:169:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:213:1)
    at invokeGuardedCallback (react-dom.development.js:261:1)
    at beginWork$1 (react-dom.development.js:20042:1)
    at performUnitOfWork (react-dom.development.js:19163:1)
    at workLoopSync (react-dom.development.js:19142:1)
    at performSyncWorkOnRoot (react-dom.development.js:18812:1)
    at react-dom.development.js:9783:1
    at unstable_runWithPriority (scheduler.development.js:572:1)
    at runWithPriority$1 (react-dom.development.js:9738:1)
    at flushSyncCallbackQueueImpl (react-dom.development.js:9779:1)
    at flushSyncCallbackQueue (react-dom.development.js:9769:1)
    at scheduleUpdateOnFiber (react-dom.development.js:18329:1)
    at Object.enqueueSetState (react-dom.development.js:11086:1)
    at push.../library/node_modules/react/cjs/react.development.js.Component.setState (react.development.js:408:1)
    at AsyncApiComponent.<anonymous> (AsyncApi.js:227:1)
    at step (AsyncApi.js:122:1)
    at Object.next (AsyncApi.js:71:1)
    at fulfilled (AsyncApi.js:30:1)
VM283:2 Uncaught ReferenceError: process is not defined
    at 4043 (<anonymous>:2:13168)
    at r (<anonymous>:2:306599)
    at 8048 (<anonymous>:2:9496)
    at r (<anonymous>:2:306599)
    at 8641 (<anonymous>:2:1379)
    at r (<anonymous>:2:306599)
    at <anonymous>:2:315627
    at <anonymous>:2:324225
    at <anonymous>:2:324229
    at e.onload (index.js:1714:1)

I checked React version and other stuff they proposed in https://legacy.reactjs.org/docs/error-boundaries.html as fix. These errors are not helpful 😞

What maintainance burden? There are only bumps needed. And the recent change from Jonas us just few lines of code.

yeah, my argument about no maintenance burden is no longer valid. Spend few hours on fixing it and did not find a solution.

@jonaslagoni Did you create the initial ticket because you knew playground is broken, and you had no fix for it? this initial info would help with discussion

@derberg
Copy link
Member

derberg commented Sep 13, 2023

nevermind 😄 just tried one small fix and it worked, as simple as

    "react": "link:../library/node_modules/react",
    "react-dom": "link:../library/node_modules/react-dom",

worth to notice that it is react version issue that we would also encounter when we would have a plain react example, not as sophisticated as playground

@jonaslagoni
Copy link
Member Author

@derberg usually this error pops up when I use npm link.

Did you use this to test the local react components?

Copy link
Member

derberg commented Sep 13, 2023

I've added it to playground instead of using separate react there. After fixing other compilation errors with starting playground, this was the only blocker to make it work. Now on local playground works without problems

@derberg
Copy link
Member

derberg commented Sep 28, 2023

btw, in the end the fix had to be:

"react": "file:../library/node_modules/react",
"react-dom": "file:../library/node_modules/react-dom",

@jonaslagoni @fmvilas so, playground works atm. Imho better to close discussion for now, as playground is harmful atm.

@jonaslagoni
Copy link
Member Author

@derberg do you mean harmless? 😄

Copy link
Member

derberg commented Oct 2, 2023

close 😃 was suppose to be not harmful 😃

Copy link

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 Jan 31, 2024
@derberg
Copy link
Member

derberg commented Feb 21, 2024

closing as playground works at the moment and there is no consensus to remove it - and there are many other much more critical issues 😅 feel free to reopen if you think it is super important issue to tackle

@derberg derberg closed this as completed Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants