-
Notifications
You must be signed in to change notification settings - Fork 22
feat(deps): Allowed support for React 19 #282
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
Conversation
// TODO: We need to replace react-measure as it doesn't support React 19. The following | ||
// casting helps to get topology to build with React 19 versions | ||
const Wrapper = ReactMeasure as any; |
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.
Just FYI, I opened a followup issue regarding this: #283
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.
Great call switching to named imports here @thatblindgeye - improves readability and keeps our bundle lean!
As we discussed, when following the testing instructions the build step has known issue with the puppeteer
package from the @patternfly/documentation-framework which is out of scope for this PR.
Is there a follow-up issue to track this necessary docs framework update? Would be great to have that tracked so we can fully enable React 19 in an upcoming task. 🚀
@jenny-s51 spoke with @kmcfaul to confirm, but at the moment there isn't anything tracking updating the current docs framework to allow extensions to upgrade to React 19 themselves. This current initiative is to unblock consumers wanting to use our packages with React 19. The new docs framework being worked on shoud hopefully not have this same issue, and we hope that will land before we plan to upgrade our packages to React 19. I imagine if we do plan to upgrade our packages to React 19 and the new dosc framework isn't ready we would open an issue then to track any necessary work, though. |
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.
Thanks for clarifying the scope @thatblindgeye @kmcfaul! Excellent, we can plan on updating our own packages to React 19 once the new documentation framework is available.
LGTM
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.
This looks good to me!
🎉 This PR is included in version 6.3.0-prerelease.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@thatblindgeye This change seems to have broken the demo app. First, it doesn't build with a failure at https://github.com/patternfly/react-topology/blob/main/packages/demo-app-ts/src/App.tsx#L38 since Second, fixing that problem leaves an issue with the initial graph not being centered. I'm not sure what is causing this. The graph is not getting the size set until after the layout is run the first time. I will look into fixing this by fixing #283 |
What
Closes #280
FYI @jenny-s51 @jeff-phillips-18 I made an update to the VisualizationSurface tsx file to cast the ReactMeasure wrapper to any, as it was failing the build process with React 19 versions. I added a TODO to replace ReactMeasure as it doesnt look like it supports React 19.
Description
Type of change
Screen shots / Gifs for design review