Skip to content

Fix for incorrect compile check#3567

Closed
vadimka123 wants to merge 3 commits intoJedWatson:masterfrom
vadimka123:fix-ensure-checks
Closed

Fix for incorrect compile check#3567
vadimka123 wants to merge 3 commits intoJedWatson:masterfrom
vadimka123:fix-ensure-checks

Conversation

@vadimka123
Copy link
Contributor

Fix check for ensure focus. Now code compiled as (isFocused && !isDisabled && prevProps.isDisabled || isFocused && menuIsOpen && !prevProps.menuIsOpen). See prettier/prettier#187

@JedWatson
Copy link
Owner

Thanks @vadimka123 but I'm not seeing the problem here. With the latest version of prettier, that code is preserved as expected, and the logic will be the same before and after your change.

Are you seeing a different behaviour?

@vadimka123
Copy link
Contributor Author

@JedWatson, I've seen this behavior before
Perhaps it is already fixed by updating prettier to latest version
I will retest and if not repeat, then close this PR

@vadimka123
Copy link
Contributor Author

@JedWatson, it's issue is not fixed
I'm check on local machine build (version of prettier - 1.17.1), also check builded code from package
Results is same, boolean logic is damaged

Снимок экрана от 2019-06-03 09-57-54

@JedWatson
Copy link
Owner

@vadimka123 Sorry for the delay, I've worked out what's going on here.

This isn't actually a prettier issue but a babel optimisation, and it's going to behave correctly because the Logical AND operator has a higher precedence (6) than the Logical OR operator (5) in JavaScript. Which means this code is safe to leave as-is, and the parenthesis grouping the logic are really just helping with code readability (as discussed on that prettier issue you linked)

reference https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence

So I'm going to close this, but I personally learned something in the process, so thank you!

@JedWatson JedWatson closed this Oct 2, 2019
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 this pull request may close these issues.

2 participants