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.

Sign up for GitHub

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

Jump to bottom

feat(Beans): Port Bean Configuration UI #166

Merged
merged 1 commit into from
Oct 2, 2023
Merged

feat(Beans): Port Bean Configuration UI #166

merged 1 commit into from
Oct 2, 2023

Conversation

igarashitm
Copy link
Contributor

Fixes: #8

It often hits the following error and fails to render. It seems the schema is not initialized at that time, possibly a race condition

React Router caught the following error during render TypeError: Cannot read properties of undefined (reading 'schema')
    at BeansPage (BeansPage.tsx:25:33)
    at renderWithHooks (react-dom.development.js:16305:18)
    at mountIndeterminateComponent (react-dom.development.js:20074:13)
    at beginWork (react-dom.development.js:21587:16)
    at beginWork$1 (react-dom.development.js:27426:14)
    at performUnitOfWork (react-dom.development.js:26557:12)
    at workLoopSync (react-dom.development.js:26466:5)
    at renderRootSync (react-dom.development.js:26434:7)
    at recoverFromConcurrentError (react-dom.development.js:25850:20)
    at performConcurrentWorkOnRoot (react-dom.development.js:25750:22) {componentStack: '\n    at BeansPage (http://localhost:5173/src/pages….vite/deps/react-router-dom.js?v=fafe9561:3597:5)'} 
    at RenderErrorBoundary (http://localhost:5173/node_modules/.vite/deps/react-router-dom.js?v=fafe9561:3287:5)
    at DataRoutes (http://localhost:5173/node_modules/.vite/deps/react-router-dom.js?v=fafe9561:3648:5)
    at Router (http://localhost:5173/node_modules/.vite/deps/react-router-dom.js?v=fafe9561:3727:15)
    at RouterProvider (http://localhost:5173/node_modules/.vite/deps/react-router-dom.js?v=fafe9561:3597:5)

@igarashitm igarashitm force-pushed the 8 branch 2 times, most recently from abd8ee2 to 0396276 Compare September 28, 2023 01:46
lordrip
lordrip reviewed Sep 28, 2023
View reviewed changes
packages/ui/package.json Outdated
@@ -88,6 +88,7 @@
"eslint-plugin-react-refresh": "^0.4.3",
"jest": "^29.4.2",
"jest-environment-jsdom": "^29.4.2",
"lodash": "^4.17.21",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding lodash as a devDependency is intended rather than a regular dependency?

I think it would be better just to bring what we need from lodash, in this case cloneDeep.

"lodash.cloneDeep" and also the types which should be "@types/lodash.cloneDeep"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there's no separate package for lodash.cloneDeep? this is 404 not found
https://registry.yarnpkg.com/lodash.cloneDeep

The reason being in dev dep is it's used only in tests.

Copy link
Member

@lordrip lordrip Sep 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the version is different but we have it in kaoto-ui 😃

https://github.com/KaotoIO/kaoto-ui/blob/872c59a2d84f2e4c74b3cd55ada8e29a400e3559/packages/kaoto-ui/package.json#L61

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhhh it's clonedeep with lower d 🤣

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK done

@lhein lhein added this to the 0.1.0 - Smart Connectors' MVP milestone Sep 28, 2023
@lhein lhein added the enhancement New feature or request label Sep 28, 2023
igarashitm
igarashitm commented Sep 28, 2023
View reviewed changes
packages/ui/src/pages/Beans/BeansPage.tsx
<Title headingLevel="h1">Beans Configuration</Title>
<MetadataEditor
name="Beans"
schema={schemaMap.beans.schema}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lordrip schemaMap.beans here is sometimes undefined, usually if I reload the browser once or twice then it loads as expected, is there a something wrong on how schema store is used?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm, I think this might be related to the initial problem, probably fetching the schemas is not a blocking operation so the beans UI gets rendered before having the schema

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK so leave it as-is, hoping that's better for prod build, or we'll revisit separately

@igarashitm igarashitm marked this pull request as ready for review September 28, 2023 16:11
@igarashitm igarashitm requested a review from a team September 28, 2023 16:11
@igarashitm
Copy link
Contributor Author

Also fixed a bug that if there's no beans in the source code, beans-editor>code sync was not working. Now it's ready I think.

lordrip
lordrip approved these changes Oct 2, 2023
View reviewed changes
Copy link
Member

@lordrip lordrip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Just a side note, I implemented these changes from uniforms-patternfly and it's available in this version: https://github.com/KaotoIO/uniforms-patternfly/releases/tag/%40kaoto-next%2Funiforms-patternfly%400.4.0

@lordrip lordrip merged commit 04af8a2 into KaotoIO:main Oct 2, 2023
@lhein lhein modified the milestones: Smart Connectors' MVP, Feature Parity Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lordrip lordrip lordrip approved these changes

Assignees

@igarashitm igarashitm

Labels
enhancement New feature or request
Projects
Kanban Board
Status: Done
Milestone
2.0.0 TP1
Development

Successfully merging this pull request may close these issues.

Port Bean configuration UI
3 participants
@igarashitm @lordrip @lhein