Skip to content
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

Allow non-booleans in the condition prop as a convenience #55

Closed
hcharley opened this issue Jul 19, 2019 · 6 comments · Fixed by #66
Closed

Allow non-booleans in the condition prop as a convenience #55

hcharley opened this issue Jul 19, 2019 · 6 comments · Fixed by #66

Comments

@hcharley
Copy link

I see a lot of these warnings:

Warning: Failed prop type: Invalid prop `children` supplied to `If`.

Because I will have conditions that look like:

<When condition={state.data && state.data.myProfile}>
  My profile loaded!
</When>

With state.data.myProfile being an object that might look like:

{
  "id": 123,
  "name": "Joe",
  "email": "[email protected]"
}

Which means that I have to do this to silence warnings:

<When condition={!!(state.data && state.data.myProfile)}>
  My profile loaded!
</When>

Or:

<When condition={state.data && state.data.myProfile && true}>
  My profile loaded!
</When>

Both of these solutions seems solvable if react-if accepted non-boolean conditions.

null and undefined or anything non-truthy would be evaluated to a false.

@romac
Copy link
Owner

romac commented Jul 23, 2019

In case you are already using Babel to compile your JSX to JavaScript, may I suggest looking into Babel's optional chaining plugin as an alternative to relaxing the prop types?

This would let you write the following instead:

<When condition={state.data?.myProfile != null}>
  My profile loaded!
</When>

@Undeadlol1
Copy link

Please. This would be awesome.
In my particular case i use create-react-app and adding babel plugins is not an option.
Also, i don't really see the reason why this components shouldn't work as an ordinary javascript "if" statement.
Manually converting to booleans is inconvenient.

@romac
Copy link
Owner

romac commented Nov 26, 2019

@Undeadlol1 Alright! I concede this could be convenient :) I cannot work on this at the moment, but feel free to send a PR with some test cases and I'll merge it.

@dehimer
Copy link

dehimer commented Dec 16, 2019

Yes, I set !! almost in all conditionals.

@isuvorov
Copy link

isuvorov commented Jan 31, 2020

+1 -- Still actual

favna added a commit that referenced this issue Oct 19, 2020
The lib has been entirely rewritten in order to have the source code in TypeScript and leveraging
the "tsdx" package for bundling, testing and linting.

BREAKING CHANGE: As part of the rewrite, the global exports are gone. Please use CommonJS or ESM
style imports as specified in the README.

fixes #55 closes #57
@favna
Copy link
Collaborator

favna commented Oct 21, 2020

This is fixed with #66 which switches the code from JavaScript using prop-types to TypeScript. Now this doesn't mean that you have to write TypeScript yourself, but PropTypes won't complain anymore about not providing a non boolean value. The new type of condition is:

export type BooleanLike = boolean | string | number | null | undefined;

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface ComponentWithConditionProps
  extends PropsWithChildren<{
    condition: (() => BooleanLike) | BooleanLike;
  }> {}

This means it can be boolean, string, number, null, undefined, () => boolean, () => string, () => number, () => null, or () => undefined

@favna favna closed this as completed in #66 Oct 21, 2020
favna added a commit that referenced this issue Oct 21, 2020
The lib has been entirely rewritten in order to have the source code in TypeScript and leveraging
the "tsdx" package for bundling, testing and linting.

BREAKING CHANGE: As part of the rewrite, the global exports are gone. Please use CommonJS or ESM
style imports as specified in the README.

fixes #55 closes #57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants