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

Make [[Delete]] on module namespace return false for @@toStringTag. #767

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

GeorgNeis
Copy link
Contributor

A module namespace exotic object has an own Symbol.toStringTag property, which
is non-configurable. This patch changes [[Delete]] to return false rather than
true for this property, thus making its behavior consistent with other own
properties of the object.

See also: #747

A module namespace exotic object has an own Symbol.toStringTag property, which
is non-configurable.  This patch changes [[Delete]] to return false rather than
true for this property, thus making its behavior consistent with other own
properties of the object.
@bterlson
Copy link
Member

This looks good to me. I'll tag it needs consensus just in case, since it's easy to discuss next week :)

@bterlson bterlson added the needs consensus This needs committee consensus before it can be eligible to be merged. label Jan 20, 2017
@bterlson
Copy link
Member

We have consensus.

@bakkot bakkot added the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Feb 13, 2017
hubot pushed a commit to WebKit/WebKit-http that referenced this pull request Feb 16, 2017
https://bugs.webkit.org/show_bug.cgi?id=168280

Reviewed by Saam Barati.

JSTests:

* modules/namespace-object-symbol-iterator-name.js:
* modules/namespace-object-typed-array-fast-path.js:
* modules/namespace.js:
(shouldBe.JSON.stringify.Reflect.getOwnPropertyDescriptor):
(shouldThrow):

Source/JavaScriptCore:

Reflect updates to the module namespace object.

1. @@iterator property is dropped[1].
2. @@toStringTag property becomes non-configurable[1].
3. delete with Symbol should be delegated to the JSObject's one[2].

[1]: https://tc39.github.io/ecma262/#sec-module-namespace-objects
[2]: tc39/ecma262#767

* runtime/JSModuleNamespaceObject.cpp:
(JSC::JSModuleNamespaceObject::finishCreation):
(JSC::JSModuleNamespaceObject::deleteProperty):
(JSC::moduleNamespaceObjectSymbolIterator): Deleted.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@212430 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@bterlson
Copy link
Member

Still need tests but I'm going to merge this.

@bterlson bterlson merged commit d7f6ed8 into tc39:master Mar 22, 2017
@GeorgNeis GeorgNeis deleted the module-namespace-delete branch May 21, 2017 17:24
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=168280

Reviewed by Saam Barati.

JSTests:

* modules/namespace-object-symbol-iterator-name.js:
* modules/namespace-object-typed-array-fast-path.js:
* modules/namespace.js:
(shouldBe.JSON.stringify.Reflect.getOwnPropertyDescriptor):
(shouldThrow):

Source/JavaScriptCore:

Reflect updates to the module namespace object.

1. @@iterator property is dropped[1].
2. @@toStringTag property becomes non-configurable[1].
3. delete with Symbol should be delegated to the JSObject's one[2].

[1]: https://tc39.github.io/ecma262/#sec-module-namespace-objects
[2]: tc39/ecma262#767

* runtime/JSModuleNamespaceObject.cpp:
(JSC::JSModuleNamespaceObject::finishCreation):
(JSC::JSModuleNamespaceObject::deleteProperty):
(JSC::moduleNamespaceObjectSymbolIterator): Deleted.

Canonical link: https://commits.webkit.org/185447@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@212430 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants