-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
lib: add cause to DOMException #44703
lib: add cause to DOMException #44703
Conversation
400012e
to
c871867
Compare
ff4713f
to
a81a794
Compare
a81a794
to
7fc758b
Compare
3c85e9a
to
004fb4c
Compare
004fb4c
to
261bbb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Thank you for working on this!
It would be great if we can have a documentation update on this. Also, a note on the behavior divergence between the (pending) WebIDL definition would be helpful, like say, experimental.
While @flakey5 is working on remembering his password to log into GitHub from his phone ;-)... When we were discussing this change we noticed that the documentation for DOMException only links to MDN. It doesn't break down any of the API. I agree we should have docs but I'd prefer to do that in a separate PR that covers all of DOMException. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Thanks.
Landed in 00e5617 |
PR-URL: nodejs#44703 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: #44703 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Adds an optional
cause
field to DOMException as per the WebIDL standard added in whatwg/webidl#1179@jasnell @panva