json-logic-js icon indicating copy to clipboard operation
json-logic-js copied to clipboard

Data is not used by "some"

Open mikol-styra opened this issue 6 years ago • 7 comments

I expected the following to evaluate to true:

Data:

{"input": "x"}

Rule:

{
  "and": [
    {
      "some": [
        [
          "x",
          "y"
        ],
        {
          "===": [
            {
              "var": ""
            },
            {
              "var": "input"
            }
          ]
        }
      ]
    }
  ]
}

If I use a literal value instead of "var": "input", it does evaluate to true:

{
  "and": [
    {
      "some": [
        [
          "x",
          "y"
        ],
        {
          "===": [
            {
              "var": ""
            },
            "x"
          ]
        }
      ]
    }
  ]
}

mikol-styra avatar Jan 03 '18 18:01 mikol-styra

thaaaat sucks. That's a design flaw, all the example cases I considered only used literal and the value being tested inside the test.

I'm open to suggestion on how to design a fix for this. One way would be to make the data being used inside the test actually a copy-and-extend of data. Like if data = {"input":"x"} then data inside the filter function is {"item":"x","input":"x"} on the first iteration and {"item":"y","input":"x"} on the second. If I patched filter like:

    }else if(op === 'filter'){
      scopedData = jsonLogic.apply(values[0], data);
      scopedLogic = values[1];

      if ( ! Array.isArray(scopedData)) {
          return [];
      }
      // Return only the elements from the array in the first argument,
      // that return truthy when passed to the logic in the second argument.
      return scopedData.filter(function(item){
          var item_and_globals = beget(data);
          item_and_globals.item = item;
          return jsonLogic.truthy( jsonLogic.apply(scopedLogic, item_and_globals));
      });

(Beget implementation is something like function beget(o){ return jQuery.extend({}, o); } )

and your rule changed to:

{
  "and": [
    {
      "some": [
        [
          "x",
          "y"
        ],
        {
          "===": [
            {
              "var": "item"
            },
            {
              "var": "input"
            }
          ]
        }
      ]
    }
  ]
}

This is based in part on how reduce uses named parameters current and accumulator

(For parity I'd also be open to using current instead of item in filter, I just wrote all my test code without considering that 🙃)

jwadhams avatar Jan 03 '18 22:01 jwadhams

This seems reasonable to me. I don’t really have a horse in the game as to whether the API should expect current or item. Assuming you choose current (as you say, for parity), it would probably be nice if all functor operations – e.g., map – also accepted {"var": "current"} to mean “the entire array element within the test.”

Thanks for the speedy response!

mikol-styra avatar Jan 04 '18 06:01 mikol-styra

One other thought: I do like the terseness of {"var": ""}. Perhaps, instead of current or whatever, the API might expect something nearly as terse. Maybe {"var": "_"}?

mikol-styra avatar Jan 04 '18 06:01 mikol-styra

Or wouldn’t you get the same effect yet be able to use {"var": ""} with the following?

      return scopedData.filter(function(item){
          var item_and_globals = beget(data);
          item_and_globals[''] = item;

mikol-styra avatar Jan 04 '18 06:01 mikol-styra

Seems like you should be able to explicitly name the iterator. That way you could do multiple levels of nesting. Here's the operation for "non-empty intersection":

{ some: ['i', [1, 2, 3, 4, 5],
  { some: ['j', [5, 6, 7, 8, 9],
    { '===': [{ var: 'i' }, { var: 'j' }]}
  ]}
]}

mhelvens avatar Mar 09 '18 17:03 mhelvens

@mhelvens does it work for you? Or it's just a concept for improvements?

alxpsr avatar Jul 29 '22 12:07 alxpsr

I am running into this exact problem as well. I see that this issue is still open, has anyone been able to come up with a workaround (other than defining custom operators)?

DylanZenoFox avatar Jan 10 '24 21:01 DylanZenoFox