-
Notifications
You must be signed in to change notification settings - Fork 67
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
FED-2298 Add ReactNode typedef #384
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
test/forward_ref_test.dart
Outdated
final ForwardRefTestComponent = forwardRef2((props, ref) { | ||
return null; | ||
}); |
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.
Another nice thing about switching from a dynamic
return type to Object?
is that for function components, you get an analysis warning if you forget to return something.
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 actually seems like a breaking change, so unfortunately I think we'll have to change the return type of JsFunctionComponent/JsForwardRefFunctionComponent and Component(2).render back to dynamic
a7a77f5
to
ead4b8e
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.
A couple comments about breakages, otherwise LGTM!
test/forward_ref_test.dart
Outdated
final ForwardRefTestComponent = forwardRef2((props, ref) { | ||
return null; | ||
}); |
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 actually seems like a breaking change, so unfortunately I think we'll have to change the return type of JsFunctionComponent/JsForwardRefFunctionComponent and Component(2).render back to dynamic
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.
+1, QAing now...
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.
- Changes look good
- Code analyzes and builds without error in over_react and other private consumers
+10
@Workiva/release-management-p
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.
+1 from RM
Motivation
In React, a "React node" is a value that can be returned from a component's render or used as children, and contains more values than just
ReactElement
.TypeScript captures this type as a
ReactNode
typeNow that Dart has type aliases (in SDK 2.13 and up), can name this type so that consumers can be more expressive in how they type their code (and also to help them avoid using
ReactElement
, which in some cases is overly restrictive).Changes
ReactNode
typedef, which is an alias forObject?
.