-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
feat: creates new playground with next.js #929
Conversation
This reverts commit 2e5873a.
Thanks so much @kennethaasan 🚀 have some initial feedback
|
|
turns out lerna was also very outdated. We were on v3, while the newest is v8. I upgraded to v8 and migrated to npm workspaces for handling dependencies and linking as recommended by lerna: https://lerna.js.org/docs/getting-started#adding-lerna-to-an-existing-repo btw, I un-deleted @derberg, please check again :) |
you are right. Since we did some fixes in playground to make it work with v3 we changed how react component is mentioned there, and it is no longer pointing to npm but with file reference to then you can remove the script and also all scripts from package.json files in playgroud and root that trigger it also please bring back |
As agreed over Slack: I brought back the Regarding package-lock, I think we should be fine by only having one in the root. I think cache-dependency-path: '**/package-lock.json' in setup-node will be fine because it is used to pick up the lock file from the root, library, playground, and web-component. Now it will pick up the root one which contains lock information for all packages. |
"scripts": { | ||
"bootstrap": "lerna bootstrap", | ||
"clean": "lerna clean", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why no longer clean is not possible? it was pretty useful during development
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm start
works like a charm now, but running build of playground gives
> build:playground
> cd playground && npm run build
> [email protected] build
> next build
▲ Next.js 14.1.0
Creating an optimized production build ...
✓ Compiled successfully
Linting and checking validity of types ...Failed to compile.
./app/page.tsx:95:14
Type error: 'AsyncApi' cannot be used as a JSX component.
Its type 'typeof AsyncApiComponent' is not a valid JSX element type.
Types of construct signatures are incompatible.
Type 'new (props: AsyncApiProps) => AsyncApiComponent' is not assignable to type 'new (props: any, deprecatedLegacyContext?: any) => Component<any, any, any>'.
Construct signature return types 'AsyncApiComponent' and 'Component<any, any, any>' are incompatible.
The types of 'refs' are incompatible between these types.
Type '{ [key: string]: React.ReactInstance; }' is not assignable to type '{ [key: string]: import("/Users/wookiee/sources/asyncapi-react/playground/node_modules/@types/react/index").ReactInstance; }'.
'string' index signatures are incompatible.
Type 'React.ReactInstance' is not assignable to type 'import("/Users/wookiee/sources/asyncapi-react/playground/node_modules/@types/react/index").ReactInstance'.
Type 'Component<any, {}, any>' is not assignable to type 'ReactInstance'.
Type 'React.Component<any, {}, any>' is not assignable to type 'import("/Users/wookiee/sources/asyncapi-react/playground/node_modules/@types/react/index").Component<any, {}, any>'.
The types returned by 'render()' are incompatible between these types.
Type 'React.ReactNode' is not assignable to type 'import("/Users/wookiee/sources/asyncapi-react/playground/node_modules/@types/react/index").ReactNode'.
93 | </CodeEditorsWrapper>
94 | <AsyncApiWrapper>
> 95 | <AsyncApi schema={schema} config={parsedConfig} />
| ^
96 | </AsyncApiWrapper>
97 | </SplitWrapper>
98 | </PlaygroundWrapper>
npm ERR! Lifecycle script `build` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: [email protected]
npm ERR! at location: /Users/wookiee/sources/asyncapi-react/playground
@vishvamsinh28 in this case it is not a problem - this PR is basically refreshing from scratch the playground that is basically a development environment only, not "users" related. So in is an exception that as maintainer I'm willing to take 😄 actually is is almost ready to merge, just need @kennethaasan to confirm if he also has the same error as I have when running |
Yeah, I have the same error. The issue seems to be that we’re using different react versions in the library and playground. I’ll look into it next week. |
@derberg I had to add some |
@@ -39,8 +39,7 @@ | |||
"scripts": { | |||
"start": "tsc --watch", | |||
"bundle": "webpack", | |||
"prepare": "npm run bundle", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll bring it back, it is still needed in https://github.com/asyncapi/asyncapi-react/blob/master/package.json#L21
will commit directly to your branch to not bother you with tiny stuff
@kennethaasan all works good now, building work like a charm last changes imho is https://github.com/asyncapi/asyncapi-react/blob/master/.github/workflows/release-wc-and-playground.yml#L89 because now, built playground ends up in you either reconfigure workflow to point to different dir, or update next config to put build files in |
@kennethaasan after bringing back
is this why you removed it initially? |
Thanks for taking another look. I removed it from web-component because it's working by luck in the master branch. The web-component prepare script requires the prepare script in library to be completed to work, and when we changed from lerna to npm workspaces, that stopped working because of bad luck 😄 I fixed it by adding a bundle step in the release web-component job. I also removed bundle from the prepack script in web-component, because that's now done in that step I added. I also changed the output folder for the playground to 🤞 that everything is okay now |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!!!
Let's merge. With 91 files changed I'm pretty sure something will have to break during the release, I can't believe we remembered about everything 😄
/rtm |
🎉 This PR is included in version 1.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This issue has been resolved in version 1.4.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Changes proposed in this pull request:
Related issue(s)
Partly fixes #891