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
[dagit] Fix runtime errors in useViewport #6246
Conversation
This pull request is being automatically deployed with Vercel (learn more). dagit-storybook – ./js_modules/dagit/packages/ui🔍 Inspect: https://vercel.com/elementl/dagit-storybook/7ttu6jJSYRNdFnUQUDmnhv254Wms [Deployment for ed91d55 canceled] dagster – ./docs/next🔍 Inspect: https://vercel.com/elementl/dagster/EErZF2FwzwowqEysZhKpExo94geR [Deployment for ed91d55 canceled] |
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
Also tidying up some deps based on yarn spew. |
6de77cf
to
87e8e30
Compare
87e8e30
to
e80f592
Compare
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, I think I was just being lazy and stashing the __sized prop on the element itself. The new null check seems safe 👍
e80f592
to
0800207
Compare
0800207
to
3652277
Compare
3652277
to
ed91d55
Compare
const suggestions = React.useMemo(() => buildSuggestions(queryString), [ | ||
buildSuggestions, | ||
queryString, | ||
]); |
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 keeps getting caught by lint runs. I believe this should iron it out.
Summary
There is an occasional Cloud runtime error within
useViewport
, where the ref is not actually an element. The ref is currently being set toany
, so there are no TS safeguards in place to prevent this.Fix this up by tracking a typed object.
Test Plan
View Gantt chart and parition matrix. Verify that they behave correctly, with no errors.