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

Operator precedence between || and && #91

Open
wasimshariff opened this issue Sep 16, 2020 · 2 comments
Open

Operator precedence between || and && #91

wasimshariff opened this issue Sep 16, 2020 · 2 comments
Assignees
Labels

Comments

@wasimshariff
Copy link

Hi Tom,

i encountered one issue with operator precedence in jexl library. Wanted to check with you before making a change at my end locally.

In grammer.js :

'&&': {
type: 'binaryOp',
precedence: 11,
eval: (left, right) => left && right
},
'||': {
type: 'binaryOp',
precedence: 10,
eval: (left, right) => left || right
}

Why does && and !! have same precedence. Usually in JS world, && has higher precedence.

Let me know, if you need any more info.

Thanks,
Wasim

@TomFrost
Copy link
Owner

TomFrost commented Sep 16, 2020

Hey Wasim,

That's a really great catch. You're absolutely right -- && has higher precedence in most languages. That's a bug in the initial design for sure.

Changing it would be a breaking change, but I can look at introducing that in a major 3.0 release. Until then, the workarounds would be to use parentheses in your expression to force precedence, or just call addBinaryOp to re-add the && operator with higher precedence. That would allow you to keep upgrading Jexl and not edit the source directly.

@TomFrost TomFrost added the bug label Sep 16, 2020
@TomFrost TomFrost self-assigned this Sep 16, 2020
@wasimshariff
Copy link
Author

Cool, thank you Tom. I cant rewrite the expression. I will patch the jexl library locally using patch-package and get rid of it when 3.x is out. Thanks

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

No branches or pull requests

2 participants