Jsonlogic import export improvement #1202
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ce42cdb:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello
I have previously mentioned my intention to refactor how JSONLogic import/export works. The main idea is to make everything work dynamically based on JSONLogic definitions found in the config, leveraging an expanded templateMatcher. My motivation was that the current implementation of JSONLogic import involves a lot of special-case logic (ie. arity etc) to recognize operators, that I found difficult to understand. I believe this new approach would make the import/export code more clear, maintainable, and less fragile while also making custom operators easier to implement. Also my previous contributions did not fully utilize the potential of the templateMatcher, and expanding its use felt relatively straightforward.
This branch is that idea mostly done. Both import and export just use the config jsonlogic definitions to figure out how to handle various operators.
You might notice from commits that I actually wrote this in December. Unfortunately I did not have time after that to really focus on this after that. As I see there are two main issues with this branch at the moment.
A test (loads tree with func SUM_OF_MULTISELECT in LHS) is failing because, in this case, the expected field object (e.g., { "var": "fieldName" }) is replaced by the custom function SUM_OF_MULTISELECT within the "between" operator. The templateMatcher relies on finding a field object in a specific location, and I'm uncertain how to handle this discrepancy. Question: Is the test incorrect, or should the code be extended to handle operators like this?
There is some issue with decompressing config. A test is failing and I have not really had time look into it.
Anyways, I have not really done anything with this since December, but if this approach seems useful I could probably finish it. Provided I get some pointers on how things are supposed to work regarding the whole sum_of_multiselect issue I mentioned. Mostly I just wanted to inform you that I made this in case you find it useful.