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

Allow to flatten same logical operations #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ketler13
Copy link

@Ketler13 Ketler13 commented Nov 5, 2020

The problem
Consider having such flat logical expression as a string:
first === 1 || second === 2 || third === 3
After transformation to the json logic it will be changed to the tree:
{"or":[{"or":[{"===":[{"var":"first"},1]},{"===":[{"var":"second"},2]}]},{"===":[{"var":"third"},3]}]}
So instead of flat structure it will be nested.
Then, if I try to use that structure with https://github.com/ukrbublik/react-awesome-query-builder the result UI will be like this:
image

Possible solution
This PR tries to flatten only logical expression node if it's left or right children are also logical expression with the same operator. Please take a look at the results after applying changes:
{"or":[{"===":[{"var":"first"},1]},{"===":[{"var":"second"},2]},{"===":[{"var":"third"},3]}]}
image

Notes
This is just an idea and maybe it is completely wrong for complex cases. Also maybe it's ok to have it in options to be able to allow/disable flattening from outside code.
Thank you for the library, it is very useful.

@juniorclearmetal
Copy link

This is a good callout, I was surprised that the given example in the readme, which is transformJS('temp < 110 && pie.filling === "apple"') works as expected but add another && block transformJS('temp < 110 && pie.filling === "apple" && iceCream === true') and now it's nested:

{
  and: [
    {
      and: [
        { '<': [ { var: 'temp' }, 110 ] },
        { '===': [ { var: 'pie.filling' }, 'apple' ] }
      ]
    },
    { '===': [ { var: 'iceCream' }, true ] }
  ]
}

Maybe there's something I'm not understanding.

@Ketler13 Ketler13 closed this Feb 1, 2021
@Ketler13 Ketler13 reopened this Feb 1, 2021
@Ketler13
Copy link
Author

Ketler13 commented Feb 1, 2021

@juniorclearmetal I believe this happens not in this library itself, but rather in underlying JS parser (I guess this is meriyah package in this case).
For this expression 'temp < 110 && pie.filling === "apple" && iceCream === true' parser will generate binary tree and there will always be left-hand and right-hand expression. Something like this:
{ op: '&&', left: temp < 110, right: pie.filling === "apple" && iceCream === true' }
and then right part will consist of:
{ op: '&&', left: pie.filling === "apple", right: iceCream === true' }
So in terms of binary tree result this expression looks correct. These are just initial thoughts, maybe it will be useful to take a look for example how prettier simplifies such kind of nested boolean expressions.

Thank you

@krismuniz
Copy link
Owner

krismuniz commented Mar 20, 2021

Goodness this PR was lost in my notifications and I never noticed @Ketler13 I'll review and merge this soon. Sorry for the huge delay

@gazben
Copy link

gazben commented Jul 23, 2021

@krismuniz Can we expect a merge on this, or is there any problem with it?

I tried out this library and as it turns out, if you want to save a really long expression without the flattening, the mysql json column fails. My workaround is, that we use more parenthesis.

@Marcisbee
Copy link

Yeah, is this PR still on the table?

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.

5 participants