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

repl: make own properties shadow prototype properties #21588

Closed
wants to merge 1 commit into from

Conversation

rubys
Copy link
Member

@rubys rubys commented Jun 29, 2018

Previously, the code displayed properties backwards (e.g., showing prototype
properties before own properties). It also did uniqueness checks during this
processing, so these checks were done backwards.

After this change, the properties continue to be displayed backwards, but
the uniqueness checks are done in the proper order.

Fixes: #15199

See also: #21586 which was discovered
during the testing of this fix.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Previously, the code displayed properties backwards (e.g., showing prototype
properties before own properties).  It also did uniqueness checks during this
processing, so these checks were done backwards.

After this change, the properties continue to be displayed backwards, but
the uniqueness checks are done in the proper order.

Fixes: nodejs#15199

See also: nodejs#21586 which was discovered
during the testing of this fix.
@nodejs-github-bot nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Jun 29, 2018
@Trott
Copy link
Member

Trott commented Jun 29, 2018

@nodejs/repl

@Trott
Copy link
Member

Trott commented Jun 30, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 30, 2018
@jasnell
Copy link
Member

jasnell commented Jul 12, 2018

CI appears to have failed across the board on this one but the results are no longer accessible. Trying again: https://ci.nodejs.org/job/node-test-pull-request/15831/

@Trott
Copy link
Member

Trott commented Jul 12, 2018

One seemingly unrelated failure on macOS 10.12. Used "Resume Build" to re-run just that in CI: https://ci.nodejs.org/job/node-test-pull-request/15835/

Also opened an issue to track the failure in case it gets seen elsewhere: #21781

@addaleax
Copy link
Member

Landed in 4d42083, thanks for the PR! 🎉

@addaleax addaleax closed this Jul 13, 2018
addaleax pushed a commit that referenced this pull request Jul 13, 2018
Previously, the code displayed properties backwards (e.g., showing
prototype properties before own properties).  It also did uniqueness
checks during this processing, so these checks were done backwards.

After this change, the properties continue to be displayed backwards,
but the uniqueness checks are done in the proper order.

See also: #21586 which was
discovered during the testing of this fix.

Fixes: #15199
PR-URL: #21588
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Jul 14, 2018
Previously, the code displayed properties backwards (e.g., showing
prototype properties before own properties).  It also did uniqueness
checks during this processing, so these checks were done backwards.

After this change, the properties continue to be displayed backwards,
but the uniqueness checks are done in the proper order.

See also: #21586 which was
discovered during the testing of this fix.

Fixes: #15199
PR-URL: #21588
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@targos targos mentioned this pull request Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"constructor" property of function prototype objects does not show up in autocomplete suggestions
6 participants