postcss / postcss-calc

PostCSS plugin to reduce calc()
MIT License
215 stars 33 forks source link

fix: incorrect reduction of subtraction from zero (#88) #93

Closed jasminexie closed 5 years ago

jasminexie commented 5 years ago

This PR should fix #88, where a division expression subtracted from zero is incorrectly reduced. Apparently reducer.js does not reduce division expressions if the denominator cannot be reduced to a value, so similar multiplication tests pass while division did not.

jasminexie commented 5 years ago

The changes I made affected only the '0 - something => -something' case, so correct me if I'm wrong please but I don't see a point of adding tests when 0 is another value. Tests like value - unreducable division expression should already be covered by value - expression and division expression tests. I have add another one if you think it hasn't been sufficiently tested.

The '0 - something => -something' case currently has the following tests:

I've added the cases:

To test the actual code I changed, the expressions on the right side should not be reducable, and they should preserve the node. One example is calc( 0 - (100vw - 10px) * 2px ). If any of these expressions are reducable, the test would implicitly fall back to the 0 - value state, which already has a test case. I've edited the PR, please tell me what you think.

alexander-akait commented 5 years ago

/cc @ben-eb can you look on this?

ben-eb commented 5 years ago

I don't have much to add really, I wasn't involved with the more recent refactoring. Thanks for the contribution, @jasminexie 😃

alexander-akait commented 5 years ago

Thanks