-
Notifications
You must be signed in to change notification settings - Fork 29k
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
assert: support symbols as assertion messages #20693
Conversation
CI: https://ci.nodejs.org/job/node-test-pull-request/14834/ EDIT (cjihrig): For future reference, CI was green. |
@cjihrig did you run into this and have a actual use case for it? Just curious because I try to find some ways to improve the messages in general a bit more. |
I did run into it accidentally. I don't personally have a use case for it, but it seems like a bug for symbols to throw here. |
Currently, assertion messages are implicitly converted to strings, which causes symbols to throw. This commit adds an explicit string conversion. PR-URL: nodejs#20693 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, assertion messages are implicitly converted to strings, which causes symbols to throw. This commit adds an explicit string conversion. PR-URL: #20693 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, assertion messages are implicitly converted to strings, which causes symbols to throw. This commit adds an explicit string conversion.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes