-
Notifications
You must be signed in to change notification settings - Fork 32
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
Operator precedence is not preserved #115
Comments
After some testing, I came up with this. But I'm not sure if it's even correct and not over engineered: diff --git src/lib/stringifier.js src/lib/stringifier.js
index 40f92f5..c58f8a9 100644
--- src/lib/stringifier.js
+++ src/lib/stringifier.js
@@ -5,6 +5,30 @@ const order = {
"-": 1,
};
+var nonAssociative = {
+ '*': false,
+ '/': true,
+ '+': false,
+ '-': true
+}
+
+function needsParenthesis(op, childOp){
+ let opOrder = order[op];
+ let childOpOrder = order[childOp];
+ if (opOrder === childOpOrder){
+ // Chains of the same operation only need parenthesis if non-associative
+ if (op === childOp){
+ return nonAssociative[op];
+ } else {
+ // Same precedence but different operator: always
+ return true;
+ }
+ } else {
+ // Follow operator precendence
+ return order[op] < order[childOp];
+ }
+}
+
function round(value, prec) {
if (prec !== false) {
const precision = Math.pow(10, prec);
@@ -19,7 +43,7 @@ function stringify(node, prec) {
const {left, right, operator: op} = node;
let str = "";
- if (left.type === 'MathExpression' && order[op] < order[left.operator]) {
+ if (left.type === 'MathExpression' && needsParenthesis(op, left.operator)) {
str += `(${stringify(left, prec)})`;
} else {
str += stringify(left, prec);
@@ -27,7 +51,7 @@ function stringify(node, prec) {
str += order[op] ? ` ${node.operator} ` : node.operator;
- if (right.type === 'MathExpression' && order[op] < order[right.operator]) {
+ if (right.type === 'MathExpression' && needsParenthesis(op, right.operator)) {
str += `(${stringify(right, prec)})`;
} else {
str += stringify(right, prec);
This does fix your case but some other existing test cases change. For now, I don't have time to figure out why/if they should change |
@mischnic Can you send a PR and we will look what we break, I can help with fixing |
I think this relates to #91 too |
Given that CSS variables are really just text replacements and they don't have implicit parenthesis around them (like variables in a mathematical sense), I think minification of expressions with |
I have same issue with I only spotted this today on production sites with our responsive iframe containers, where we set an inline HTML <div class="responsive-media" style="--aspectRatio: 560/315;">
<iframe width="560" height="315" ...></iframe>
</div> .responsive-media {
padding-top: calc(100% / (var(--aspectRatio)));
} Output in DEV mode with padding-top: calc(100% / (var(--aspectRatio))); Output in PROD mode after with padding-top: calc(100% / var(--aspectRatio)); |
Same problem here. This bug is serious since it completely breaks calculation then the interface. |
I've given up waiting for a fix for this. I'm now using the new standard CSS |
Source:
7.0.4 outputs:
The output should instead be:
postcss-calc is removing the necessary parentheses and affecting the operator precedence.
The text was updated successfully, but these errors were encountered: