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

Implementing Object.values and Object.entries #41

Closed
wants to merge 1 commit into from
Closed

Implementing Object.values and Object.entries #41

wants to merge 1 commit into from

Conversation

akroshg
Copy link
Contributor

@akroshg akroshg commented Jan 9, 2016

These ES7 features are implemented as per the ES7 proposal https://github.com/tc39/proposal-object-values-entries/blob/master/spec.md.
The implementation is predominantly based on the result of the Object.keys.
unittest is added for more information about the usage.

@msftclas
Copy link

msftclas commented Jan 9, 2016

Hi @akroshg, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Akrosh Gandhi). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@akroshg
Copy link
Contributor Author

akroshg commented Jan 9, 2016

@abchatra - could you reviews this? thanks,

Assert(propertyId != Constants::NoProperty);

Var value = JavascriptOperators::GetProperty(object, propertyId, scriptContext);
if (!valuesToReturn)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't see anywhere you're checking enumerability in the loop (in either function) - if, during iteration, an unvisited property is deleted or made non-enumerable, it should be skipped. See tc39/test262#453 for the test262 tests.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having properties deleted and their attributes changed during enumeration would be an excellent unit test to add.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bterlson bterlson changed the title Implementing Object.keys and Object.entries Implementing Object.values and Object.entries Jan 13, 2016
@dilijev
Copy link
Contributor

dilijev commented Jan 13, 2016

@dotnet-bot test this please

@ljharb
Copy link
Collaborator

ljharb commented Jan 18, 2016

How's this one coming?

@abchatra
Copy link
Contributor

@ljharb Akrosh is out of office and will be back next week to complete this PR.

@akroshg
Copy link
Contributor Author

akroshg commented Jan 21, 2016

I am creating new PR for this one (as my fork was created on the private branch). please see #171.

@dilijev
Copy link
Contributor

dilijev commented Jan 22, 2016

Migrated to #171

@dilijev dilijev closed this Jan 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants