-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
process: deprecate toStringTag assignment #26717
Conversation
Yeah, that’s what I’d expect. If we do want to change this, I’d prefer a clean break. |
2b7d664
to
29b9483
Compare
I'm a bit reluctant to land this before Node 12. This will break a lot of projects that are not using the latest version of Jest. |
@@ -2361,6 +2361,15 @@ changes: | |||
description: Runtime deprecation. | |||
--> | |||
|
|||
<a id="DEP0126"></a> |
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.
- This seems to break other deprecation section without adding its own description.
- We usually assign a placeholder
DEP0XXX
till landing. - PR URL should be
https://github.com/nodejs/node/pull/26717
It would just warn them unless they update Jest yes. |
This does not seem something urgent, so I suggest to postpone landing this after v12. I am fine with a deprecation warning or a clean break, what ever others feel like is best in this case. |
@BridgeAR agreed, that makes sense. I won't continue to push this - shall we leave it open for anyone to pick up in future, or shall I just close? |
The release is not that far, so I just marked this as blocked. |
@guybedford @BridgeAR Looks like this is something that we can unblock now and get it to a closure? |
@guybedford is this something you would like to continue pursuing? This is definitely unblocked by now. It needs a rebase though. |
@guybedford, This need a rebase and |
8ae28ff
to
2935f72
Compare
This one has dropped off my radar, closing but someone else is welcome to pick it up. |
As a follow-on to #26488 this makes writing to
process[Symbol.toStringTag]
a full deprecation in Node.js.Most intrinsics with
Symbol.toStringTag
set (egPromise.prototype
), have this as non-writeable, so it seems to make sense thatprocess
should do the same.I'm not strongly set on this by any means either if anyone has reasons against it (perhaps the warnings will turn out more annoying than the strict error previously!).
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes