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

fix: Add check to pkg command to deal with empty values #6902

Merged
merged 14 commits into from
Oct 16, 2023
18 changes: 8 additions & 10 deletions lib/utils/queryable.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,14 @@ const getter = ({ data, key }) => {
}, {})
return _data
} else {
// if can't find any more values, it means it's just over
// and there's nothing to return
if (Object.hasOwn(_data, k) && !_data[k]) {
_data = ''
} else if (!_data[k]) {
return undefined
} else {
// otherwise sets the next value
_data = _data[k]
}
const value = _data[k]
if (typeof value === 'boolean' || (value === '' && Object.hasOwn(_data, k))) {
_data = value
} else if (value === undefined) {
Copy link
Member

@wraithgar wraithgar Oct 16, 2023

Choose a reason for hiding this comment

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

if value === undefined and value = _data[k] then returning _data[k] would return undefined anyways. Not sure what this check gains us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I did overcomplicate that. The undefined check is needed since other functionality/tests expect it to be returned and not just set to _data.

I simplified it so now it only returns undefined, else just sets up _data = value

Copy link
Member

@wraithgar wraithgar Oct 16, 2023

Choose a reason for hiding this comment

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

> The undefined check is needed since other functionality/tests expect it to be returned and not just set to _data.

Can you show me where this is happening? That still seems very counterintuitive.

Copy link
Member

Choose a reason for hiding this comment

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

oooooh I see it now. we're not returning otherwise. My bad!

Copy link
Member

Choose a reason for hiding this comment

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

OK I think we're getting somewhere. I think this check should be JUST a hasOwn, and if that's false we return undefined. Otherwise we let it fall through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works, change has been made

return undefined
} else {
_data = _data[k]
}
}

label += k
Expand Down