-
Notifications
You must be signed in to change notification settings - Fork 113
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
Fix for Plotly chart not rendering #1701
Conversation
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.
Amazing!! This has been a mind-boggling issue. We even faced this last year when building out metric plots. So glad it is solved.
Thanks a lot @jitu5
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 one!
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.
Looking great and works well !! Nice to have this in the release note. Thank you @jitu5
@@ -3,6 +3,7 @@ import RunDataset from '.'; | |||
import { runs, trackingData } from '../../experiment-wrapper/mock-data'; | |||
import JSONObject from '../../json-object'; | |||
import { shallow, mount } from 'enzyme'; | |||
import 'core-js/stable/structured-clone'; |
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.
Is this required ?
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.
Yes, structuredClone is not defined for jest test environment using NodeJS version below 17.0.0, Here its being pulled from core-js
@@ -77,7 +77,12 @@ const RunDataset = ({ | |||
trackingData, | |||
theme, | |||
}) => { | |||
if (!trackingData) { | |||
const clonedTrackingData = useMemo( | |||
() => structuredClone(trackingData), |
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 !!
Description
Resolves issue under experiment tracking where Plotly graph is not rendering.
Development notes
Considering the nested nature of the
trackingData
, it could be a data immutability issue causing the problem. To address this, I created a deep copy of thetrackingData
usingstructuredClone
and passed it to the<PlotlyChart>
component.Checklist
RELEASE.md
file