-
-
Notifications
You must be signed in to change notification settings - Fork 32.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
[DialogTitle] Test scenario where children are a string #6386
[DialogTitle] Test scenario where children are a string #6386
Conversation
agamrafaeli
commented
Mar 19, 2017
- PR has tests / docs demo, and is linted.
- Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
- Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).
I'm gonna accept that PR as the work has already been done. But I don't think it's useful to test the string variation. Thanks. |
Testing the string variation is important on a large scale project because you could get an exception there and never know it. |
@agamrafaeli Sure, but from my perspective, it feels like asserting |
@mikew No link with this PR. Still, that diff is coming from that logic: {typeof children === 'string' ? (
<Text type="title">
{children}
</Text>
) : children} I think that it would be better to change the switch logic by something more deterministic, like having a |
Not sure what you mean, I was imagining areAnyChildrenStrings = Array.isArray(children)
? children.filter(x => typeof x === 'string').length > 0
: typeof x === 'string' And then the statement you copied above, but branching off of |
Thanks @oliviertassinari |