5.0.4 includes a breaking change.

[jwb]

Describe the bug

After installing 5.0.4, code that works with 5.02 & 5.0.3 encounters an exception.

Code sample or steps to reproduce

import {JSONPath} from 'jsonpath-plus';

describe("The JSONPath function", () => {
    it("allows the search to scan entries with a null value", () => {
        expect(JSONPath({json: {content: null}, path: "$..[?(@.value)]"})).toBeTruthy()
    });
})

Console error or logs

      TypeError: Cannot read property 'value' of null
          at evalmachine.<anonymous>:1:6

Expected behavior

Versions 5.0.2 and 5.0.3 allow conditional selectors to reference attributes of @, since it is always assigned to a node.

Expected result

Tests pass.

Environment (IMPORTANT)

• JSONPath-Plus version: 5.0.4

Desktop**

• OS: OSX 11.2.2
• node: 14.13.1

[brettz9]

Yeah, I sympathize, but I'm thinking this was not such much of a breaking change, as a bug that never should have been allowed to occur as it did.

The path really should be $..[?(@ && @.value)] per the pattern currently established.

In our (and the original jsonpath) implementation at least, filters are not mere syntax of our own choosing--they evaluate as JavaScript, so I think JavaScript behavior is reasonable to be expected here ("if it might not exist, be sure to check for its existence").

So I think this probably should stay as is, and users would just have to refactor their code for now.

On the other hand, I am strongly in favor of moving to align ourselves to the kind of (consensus-based) standardization process set up by https://github.com/cburgmer/json-path-comparison , so I'd suggest bringing this up on that repo to see what consensus brings, as I don't currently see a quite analogous test yet existing on the comparison demo. If doing so, please note that our implementation does (now) allow @ to be used in the likes of $..*[?(@ === false)] (against values on object properties) so we wouldn't want to lose the ability just added to make such a distinction (though the comparison site does seem to show we have a non-standard, erring behavior when the same is applied to arrays).

My general principle is that I really wouldn't want to make any changes which lose expressiveness--being able to check for undefined, false, etc. would be somewhat of a non-starter for my tastes--unless there really was a strong consensus perhaps, but even then, I'd probably want a non-default option.

[brettz9]

Also, btw, I presume you didn't mean for the test to try to find a truthy result, but meant for it to be empty.

[jwb]

It seems that this is a good reason to start 6.0. In what sense is it not a breaking change? If there is some specification that it's striving to meet, breaking changes along the way still warrant major version bumps, if you care for the folks who rely on your work.

Reading the material on jsonpath didn't leave me with any expectation that null is a valid state for @. Much of the documentation refers to XPath, which doesn't require conditionals to test for null nodes (I don't recall any way to represent a null node in XML).

[brettz9]

Fixes inevitably break things for those who are not following the pattern that was originally meant to be enforced. This is a well-established pattern; this is not an explicit contradiction with semver in projects (some popular projects, like ESLint, are are even more loose with the understanding). Admittedly we did have one test which was following the old pattern, but that was an oversight.

If you want to be 100% sure there is no breakage under any circumstances, then pin your packages to the precise (patch) version you want (and have tests/coverage for the conditions you want). There are tools like npm-check-updates to auto-update one's dependencies in bulk (or conditionally) to the latest version, so one need not lose convenience during development to try against the latest before updating.

null is just another value in JavaScript, so it makes sense it can be null (and beyond that we explicitly support undefined which is even less of a value). The comparison to XPath is there for convenience.

[jwb]

My experience is that fixes generally fix things. A good way to introduce this behavior would be with a new feature (in 5.1.0) that allows opt-in to the new feature via a new option in JSONPathOptions ('traverseIntoFalseyNodes', perhaps). This could include the warning that 6.0.0 will have that behavior and code should migrate to using it.

Certainly ESLint is not shipped in production code. I was hoping that jsonpath-plus would be reliable enough to ship in production.

Please let me know if you would like a PR to implement an opt-in transition to the new behavior that you intend.

[brettz9]

My experience is that fixes generally fix things.

There is no need to be sarcastic. I have already explained why this is indeed a fix from our existing perspective. We have made regular breaking changes as major releases previously which were actual breaks from deliberate but outdated past choices.

As far as ESLint, I was not suggesting it was equivalent. Other production projects will make fixes that break bad behavior.

And making a fix now will indeed be making it break from those using the recent fix.

I'm sorry for your situation, but ending discussion here. If you feel inclined to work with the above-mentioned project to get a standard behavior going, feel free to file a new issue linking to the results of that discussion.