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: Handling console.log(null) #1520

Merged
merged 2 commits into from
Nov 4, 2024
Merged

Fix: Handling console.log(null) #1520

merged 2 commits into from
Nov 4, 2024

Conversation

morganick
Copy link
Contributor

@morganick morganick commented Nov 2, 2024

Please verify the following:

  • yarn build-and-test:local passes
  • I have added tests for any new features, if relevant!
  • README.md (or relevant documentation) has been updated with your changes

Describe your PR

Fixes #1517 where calling console.log with null resulted in a blank screen. Now the comments around the function say that it only handles string, object, number, and boolean; however, the function takes a message of type any. Seems like we could narrow that down a bit. I've checked with all of the possible return values for typeof and documented that in the code.

The reason the bug existed is because typeof null results in object. The code -- nor many people-- were expecting that. Here's the reference to the docs as to why: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/typeof#typeof_null

It appears that we do some things to not return functions as strings and we don't handle Symbol. Not sure if that's a requirement based on what the comments around the function say that it should accept as a message type.

⚠️ I'm not sure why the getPreview function returned just the raw message if we didn't discern what type it was. That seems like a bug. If someone has context as to why that was the case, I'd love to understand why.

Screenshot

This is what it looks like now:

image

Here's another case where we didn't handle null correctly and now we do:

image

Happy Debugging 😎

@morganick morganick self-assigned this Nov 2, 2024
@morganick morganick added the bug 🪲 Nope, this is wrong. label Nov 2, 2024
@morganick morganick marked this pull request as ready for review November 2, 2024 18:50
@morganick morganick merged commit 98e4805 into master Nov 4, 2024
3 checks passed
@morganick morganick deleted the fix-console-log-null branch November 4, 2024 12:56
@jamonholmgren
Copy link
Member

That's a wild bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Nope, this is wrong.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logging null crashes the app UI
4 participants