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

Fix valid + falsy propvalues #563

Merged
merged 7 commits into from
Aug 28, 2016
Merged

Fix valid + falsy propvalues #563

merged 7 commits into from
Aug 28, 2016

Conversation

ljharb
Copy link
Member

@ljharb ljharb commented Aug 26, 2016

Fixes #562.

I also consolidated some code.

@nfcampos
Copy link
Collaborator

is '' a valid prop value as well?

@ljharb
Copy link
Member Author

ljharb commented Aug 27, 2016

@nfcampos good call, i'll cover that also.

@ljharb ljharb changed the title Fix zero propvalue Fix valid + falsy propvalues Aug 27, 2016
@nfcampos
Copy link
Collaborator

and while you're at it null and NaN as well?

@ljharb
Copy link
Member Author

ljharb commented Aug 27, 2016

Are null and NaN actually selectable propValues with the string selector syntax? This PR is quickly growing large in scope…

@ljharb
Copy link
Member Author

ljharb commented Aug 27, 2016

Doesn't React strip out null properties?

@nfcampos
Copy link
Collaborator

only undefined I believe
https://esnextb.in/?gist=899331d29dda61430d88278ddb44a5d8

@ljharb
Copy link
Member Author

ljharb commented Aug 27, 2016

Looks like it doesn't strip out false either.

@ljharb
Copy link
Member Author

ljharb commented Aug 28, 2016

Ready for a stamp!

return propValue;
}

if (is('NaN', propValue)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason this isn't propValue !== 'NaN'?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean ===

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll change it; i'd been using a literal NaN and then updated it to take in a string

@ljharb ljharb force-pushed the fix_zero_propvalue branch 2 times, most recently from d00357b to 58c58ca Compare August 28, 2016 16:39
@nfcampos
Copy link
Collaborator

LGTM

@ljharb ljharb merged commit b566033 into master Aug 28, 2016
@ljharb ljharb deleted the fix_zero_propvalue branch August 28, 2016 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants