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

Add explicit support for length attribute #224

Closed
wants to merge 1 commit into from

Conversation

gerad
Copy link

@gerad gerad commented Oct 11, 2024

PR description

The length attribute works by happy accident today. This adds initial explicit support for it and also gets it work with filter conditions.

This is not complete support. It won't (for instance) work correctly within array filter conditions. See the test changes for use cases it does add support for.

Note that the ietf standardization for length() and count() functions is done differently.

See: https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/draft-ietf-jsonpath-base.html#name-length-function-extension

Fixes #202

Checklist

  • - Added tests
  • - Ran npm test, ensuring linting passes
  • - Adjust README documentation if relevant

The length attribute works by happy accident today. This adds initial explicit
support for it and also gets it work with filter conditions.

This is not complete support. It won't (for instance) work correctly within
array filter conditions. See the test changes for use cases it does add support
for.

Note that the ietf standardization for length() and count() functions is done
differently.

See: https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/draft-ietf-jsonpath-base.html#name-length-function-extension

Fixes JSONPath-Plus#202
const res0 = result[0];
result = [{
path: push(res0.path, 'length'),
value: result.length,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to flatten here?

@@ -260,15 +260,31 @@ JSONPath.prototype.evaluate = function (
if (exprList[0] === '$' && exprList.length > 1) {
exprList.shift();
}
let wantsLength = false;
if (exprList.at(-1) === 'length') {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think treating .length as length function is a bad idea, it will conflict with length property, if the object has it.

@brettz9
Copy link
Collaborator

brettz9 commented Oct 11, 2024

I think we really need to freeze development which does not align with the IETF spec (unless that spec is truly deficient in some way and in such a case, follow the jsonpath comparison project as possible for the way forward).

@gerad
Copy link
Author

gerad commented Oct 11, 2024

Thanks for the fast feedback both! Good point on objects with a length property.

Will revisit.

@gerad gerad closed this Oct 11, 2024
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.

continue to deal the result as a array after filter
3 participants