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

useBooleanShorthandSyntax strip false values #732

Open
pascalduez opened this issue May 17, 2022 · 3 comments
Open

useBooleanShorthandSyntax strip false values #732

pascalduez opened this issue May 17, 2022 · 3 comments

Comments

@pascalduez
Copy link
Contributor

The useBooleanShorthandSyntax option (which is on by default), strips false values.

While the behavior is fine for a true value: <Foo bar={true}> --> <Foo bar>,
it's not for a false value: <Foo bar={false}> --> <Foo>,
which is definitely not the same thing, the prop is removed completely.

@armandabric
Copy link
Collaborator

Hi @pascalduez thanks for reporting this case.

I see your point, the actual behavior came from the class component: we could set a default value on the for the props. We use there default value to know if the falsy boolean props should be shown. In that case it works as expected.

Here the class component we use to test this behavior:

class DefaultPropsComponent extends React.Component {}
DefaultPropsComponent.defaultProps = {
test: 'test',
boolean: true,
number: 0,
undefinedProp: undefined,
};

And the actual test:

it('should render boolean props if value is `false`, default is `true` and "showDefaultProps" is false', () => {
expect(
reactElementToJSXString(<DefaultPropsComponent boolean={false} />, {
showDefaultProps: false,
})
).toEqual('<DefaultPropsComponent boolean={false} />');
});

That being said, in the actual react word we could said that most of the components are functional one. We could try to change this behavior to show false props if no default value are set. I will give a try

@pascalduez
Copy link
Contributor Author

Hey @armandabric,

thanks for your reply.
I'm not sure to fully grasp, but this should be decorrelated from class vs functional components I guess.
You can set "default props" on functional components as well, and in two ways.

  • Wit the defaultProps "static" property
  • With default parameters

@rmarquois
Copy link

rmarquois commented Jan 24, 2024

I created a patch from @armandabric PR and it works.
Like @pascalduez said, I think library should handle the two ways to define defaultProps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants