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 type error when response is missing included #83

Closed
wants to merge 1 commit into from

Conversation

pelargir
Copy link
Collaborator

This fixes the following error when the response from the server does not have the included key:

TypeError: Cannot read properties of undefined (reading 'included')

When missing, the value already defaults to an empty array so this should be safe when it's "undefined" as well.

This fixes the following error when the response from the server does
not have the `included` key:

TypeError: Cannot read properties of undefined (reading 'included')

When missing, the value already defaults to an empty array so this
should be safe when it's "undefined" as well.
@pelargir pelargir requested a review from tvdeyen June 10, 2024 18:10
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

If I read the error message correctly the response object is undefined and this does not have the included key - what's expected, because the response is null/undefined.

I don't think it's the responsibility of the deserialization code to check if the response is null/undefined. This should be handled in the response handler of your Ajax/fetch call.

@pelargir
Copy link
Collaborator Author

You're right. This happens when the response is null. It doesn't have anything to do with the included key.

I think good defensive code that gets called from outside the library should absolutely check for the presence of a response before acting upon it, but I understand why you might disagree.

@pelargir pelargir closed this Jun 12, 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.

2 participants