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

Update obsolete note on duplicate Proxy [[OwnPropertyKeys]] entries #925

Merged

Conversation

shvaikalesh
Copy link
Contributor

@shvaikalesh shvaikalesh commented Sep 21, 2020

The note was added in #180, a few months before tc39/ecma262#833 was merged.

cc @domenic @bzbarsky


Preview | Diff

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Sep 21, 2020

Technically, this can happen with USVString keys which contain unpaired surrogates, but that applies to ordinary objects as well.


So, the note should be:

Note: it's possible that |typedKey| is already in |result|, if |K| is {{USVString}} and |key| contains unpaired surrogates.

Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com>
@shvaikalesh shvaikalesh force-pushed the remove-obsolete-proxy-keys-note branch from e1734b9 to ec9e010 Compare October 15, 2020 09:18
@shvaikalesh shvaikalesh changed the title Remove obsolete note on duplicate Proxy [[OwnPropertyKeys]] entries Update obsolete note on duplicate Proxy [[OwnPropertyKeys]] entries Oct 15, 2020
@TimothyGu TimothyGu merged commit 67bc60f into whatwg:master Oct 16, 2020
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 16, 2023
The comment states that duplicate key names can occur due to a proxy
object, but it doesn't appear to be the case as of current ES262 ---
[[OwnPropertyKeys]] for objects is guaranteed not to have duplicates:
https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys
(See steps 9, 13, and the invariants note afterwards)

Instead, switch the comment to talk about the case the WebIDL spec mentions --- USVString keys with unmatched surrogates.

(This follows a 2020 change to WebIDL:  whatwg/webidl#925, which follows a 2017 change to the language).

Change-Id: Ifb2bff0ca548d1a6a9d278d51eca8e204a522e59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4780986
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184129}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants