-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
EnvDeleter should return true #7960
Comments
I'm inclined to say it's a V8 bug because node.js follows the contract for delete interceptors to the letter. From the doc comment for
|
Yes, the callback should return true 'if the property could be deleted'. The question is: what does it mean that it could be deleted? In JavaScript world this means that the property should not be non-configurable (and it may not even exist). Compare it with
that returns |
Can you open a V8 bug? I'd like to hear what they have to say. |
OK, I have filled https://bugs.chromium.org/p/v8/issues/detail?id=5260 |
We can use |
@fhinkel What would you do in that case though? Users can't freeze/seal |
I misunderstood, I thought the issue is, that we should throw. If |
According to TC39 specification, the delete operator returns false or throws in strict mode, if the property is non-configurable. It returns true in all other cases. Process.env can never have non-configurable properties, thus EnvDelete must always return true. This is independent of strict mode. Fixes nodejs#7960
Thank you for a quick fix! |
According to TC39 specification, the delete operator returns false or throws in strict mode, if the property is non-configurable. It returns true in all other cases. Process.env can never have non-configurable properties, thus EnvDelete must always return true. This is independent of strict mode. Fixes: #7960 PR-URL: #7975 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Consider the following test-case:
It is weird to see
delete
operator to returnfalse
it strict-mode. It should returntrue
or throw an error there. While this seems to be also a bug in V8 (it should throw an error when the deleter callback returns false in strict-mode) the implementation ofEnvDeleter
(insrc/node.cc
) should be fixed as well. It should returntrue
for any environment variable (even the non-existing ones) to match the behaviour ofdelete
operator that is supposed to returntrue
unless non-configurable property is being deleted.The text was updated successfully, but these errors were encountered: