Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Throw error if graphql response field is null #59

Closed
wants to merge 1 commit into from

Conversation

stergion
Copy link

Some times a graphql response fields have value null. This causes currentValue.hasOwnProperty(searchProp) to throw TypeError: Cannot read properties of null (reading 'hasOwnProperty') error. The fix checks first if currentValue is null before calling hasOwnProperty() method.

Resolves #58


Behavior

Before the change?

  • Throughs TypeError if graphql response field has value of null.

After the change?

  • Checks if field is null to prevent TypeError

Some times a graphql response fields have value null. This causes currentValue.hasOwnProperty(searchProp) to throw `TypeError: Cannot read properties of null (reading 'hasOwnProperty')` error. The fix checks first if `currentValue` is null before calling hasOwnProperty() method.
@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented label Feb 12, 2023
@kfcampbell
Copy link
Member

Some times a graphql response fields have value null.

Are you saying this is intended behavior and not an API issue?

@stergion
Copy link
Author

I'm not sure if this is an API issue or not, although it seems like it.
Some fields that I have checked, like name, location, bio, that return null are of type String, so returning an empty string should be the better option.

Isn't it better though checking for null anyway?
These were just the fields that I found. There might be many more fields that return null and some may not be able to change them.

@nickfloyd
Copy link
Contributor

Hey @stergion,

Thanks for doing this ❤️ . Would you mind writing some test coverage for the null check as we'll? That should allow future contributors to understand why it was put in place and also makes sure we don't inadvertently break things in the future.

@stergion stergion closed this by deleting the head repository Oct 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Type: Bug Something isn't working as documented
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

[BUG]: deepFindPathToProperty() throws TypeError if graphql response field is null
4 participants