Skip to content

wrap option returns the same result for arrays whether true or false #86

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

Open
wal5hy opened this issue Aug 27, 2018 · 3 comments
Open
Labels
Milestone

Comments

@wal5hy
Copy link

wal5hy commented Aug 27, 2018

Given the following JSON:

  {
    "store": {
      "book": [
        {
          "category": "reference",
          "author": "Nigel Rees",
          "title": "Sayings of the Century",
          "price": 8.95
        },
        {
          "category": "fiction",
          "author": "Evelyn Waugh",
          "title": "Sword of Honour",
          "price": 12.99
        }
      ]
    }
  }

JSONPath returns the same result for the following 2 queries:
JSONPath({data: data, path: "$.store.book", wrap: true});
JSONPath({data: data, path: "$.store.book", wrap: false});

[
  [
    {
      "category": "reference",
      "author": "Nigel Rees",
      "title": "Sayings of the Century",
      "price": 8.95
    },
    {
      "category": "fiction",
      "author": "Evelyn Waugh",
      "title": "Sword of Honour",
      "price": 12.99
    }
  ]
]

I believe for wrap: false, the outer array should be removed like this:

  [
    {
      "category": "reference",
      "author": "Nigel Rees",
      "title": "Sayings of the Century",
      "price": 8.95
    },
    {
      "category": "fiction",
      "author": "Evelyn Waugh",
      "title": "Sword of Honour",
      "price": 12.99
    }
  ]
@brettz9 brettz9 added the Bug label Oct 19, 2018
@brettz9
Copy link
Collaborator

brettz9 commented Oct 20, 2018

This would be the more intuitive behavior.

However, wrap has worked as follows...

  1. When no results are found:
    1. If wrap is true (the current default), return an empty array.
    2. If wrap is false, return undefined.
  2. When a single non-array result is found:
    1. If wrap is true (the current default), follow case 3 .
    2. If wrap is false, return the item as is.
  3. When more than one result or an array is found, return the results within an array (regardless of wrap).

Thus if you get array results with wrap: false, you know you either grabbed an array or got multiple results and can just use the results as is, without any unwrapping. You know with undefined that nothing was found, and if you got a scalar, that that was the single value.

A better naming for this alternative wrap: false (and I think preferable) behavior might be unwrap: true.

Perhaps we could add an option that avoids breaking existing code and follows the behavior you describe (of unwrapping arrays as well): wrap: 'never'.

And perhaps we should also add an option of wrap: 'found' with this behavior:

  1. When no results are found, return undefined.
  2. For any other case, return the results wrapped within an array

This approach would have the benefit over wrap: false of allowing a single approach with non-array or array results and single or multiple results. It would also have the benefit over wrap: false (and wrap: true) of being able to distinguish between finding the result undefined and not finding any result.

(Note the responsible code to change is at https://github.com/s3u/JSONPath/blob/master/src/jsonpath.js#L202-L214 , esp. 203-205)

@richturner
Copy link

Just my thoughts/observations...

Point 3 above about how wrap has worked doesn't match what I am seeing; I see an array where the first item is also an array (the one result)...ending up with a multidimensional array is odd and don't think unwrapping the result array is anyway required, if you're using an expression with the expectation of getting an array then you do whatever you need to with the returned result in your own code doesn't seem like the responsibility of this library.

Ideally wrap: false should be consistent in that it returns the result (if single match); if you are going to change the data type of wrap anyway then it is already a breaking change so why not avoid the confusion of what the different wrap values mean and just keep it as boolean with predictable behaviour.

@brettz9
Copy link
Collaborator

brettz9 commented Oct 5, 2019

Thanks for your comments. I'll give this some more thought if I'm revisiting this, but tbh, this library is a low priority for me atm.

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

3 participants