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

Path for array includes to contain key #385

Merged
merged 7 commits into from
Aug 27, 2014
Merged

Path for array includes to contain key #385

merged 7 commits into from
Aug 27, 2014

Conversation

danielb2
Copy link
Contributor

Current behavior for an array including an object leaves the path without a key in the object.

A message will read correctly: "message: 'x position 1 fails because x must be a number"
While the corresponding path is: "x.1"
This fix changes the path to read: "x.1.x" which then also matches the error message.

This also matches the array behavior with how objects handle the same; where the final key is referenced that has the error occurring.

Assumptions: There will only be one reason so the error will be the first in the array (hence context.reason.0) . I couldn't find any case where it would be anything else.

@danielb2 danielb2 added the bug label Jul 23, 2014
@@ -40,6 +40,22 @@ internals.Err.prototype.toString = function () {
};


internals.Err.prototype.getPath = function () {
Copy link
Member

Choose a reason for hiding this comment

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

May want to move this off the prototype and into an internals... otherwise it may be a little odd to have item.path and item.getPath()

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to expose this as a new API. Just pass this as an argument.

@geek geek added this to the 4.7.0 milestone Aug 22, 2014
geek added a commit that referenced this pull request Aug 27, 2014
Path for array includes to contain key
@geek geek merged commit 760a694 into hapijs:master Aug 27, 2014
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants