-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
chore: migrate ErrorBoundary component from jsx to tsx #17908
chore: migrate ErrorBoundary component from jsx to tsx #17908
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17908 +/- ##
==========================================
- Coverage 67.08% 66.34% -0.74%
==========================================
Files 1609 1568 -41
Lines 64897 61502 -3395
Branches 6866 6241 -625
==========================================
- Hits 43533 40803 -2730
+ Misses 19498 19101 -397
+ Partials 1866 1598 -268
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@etr2460 Ptal, thx! Also, Happy New Year. |
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.
2 questions and a comment
@@ -82,7 +82,7 @@ const LeftSideContent = styled.div` | |||
|
|||
interface ErrorAlertProps { | |||
body: ReactNode; | |||
copyText?: string; | |||
copyText?: string | JSX.Element; |
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 change intentional?
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.
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.
Type for copyText
becomes invalid if type JSX.Element
is not set. Following error gets thrown in the ErrorMessageWithStack
Component and the index.tsx
file eventually:
(JSX attribute) copyText?: string | undefined
Type 'Element' is not assignable to type 'string'.ts(2322)
ErrorMessageWithStackTrace.tsx(33, 3): The expected type comes from property 'copyText' which is declared here on type 'IntrinsicAttributes & Props'
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.
Gotcha, thanks!
@@ -30,8 +30,8 @@ type Props = { | |||
error?: SupersetError; | |||
link?: string; | |||
subtitle?: React.ReactNode; | |||
copyText?: string; | |||
stackTrace?: string; | |||
copyText?: string | JSX.Element; |
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.
same question here
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.
Same reason as above
00b7706
to
95609cc
Compare
95609cc
to
b36ca8b
Compare
@@ -82,7 +82,7 @@ const LeftSideContent = styled.div` | |||
|
|||
interface ErrorAlertProps { | |||
body: ReactNode; | |||
copyText?: string; | |||
copyText?: string | JSX.Element; |
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.
Gotcha, thanks!
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue |
SUMMARY
PR for migrating
ErrorBoundary
class based react component from JSX to TSXBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
SOURCES REFFERED
error
andinfo
used as state properties in react error boundary