-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Revert "[New] support enumerable Symbol properties" #6
Conversation
…port enumerable Symbol properties" This partially reverts commit 7d659e7. Co-authored-by: Martin Pražák <martin.prazak@lutherone.com> Co-authored-by: Jordan Harband <ljharb@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A naive revert isn't the best solution here; it'd be to put the symbol part behind an option (ie, (includeSymbols ? ownEnumerableKeys : objectKeys)
everywhere ownEnumerableKeys
is currently called)
@ljharb I added options to traverse. It now only deals with string keys by default. When given Also added |
(you'll also need to rebase; i pulled out the immutable → options refactor) |
@ljharb I changed the jsdocs as requested, check for the existance of higher order, otherwise, the code uses native javascript. Rebased your changes, the tests run successfully, all should be good. |
The |
Don’t worry about the automatic rebase workflow; it’s failing because you made your PR from “main”. |
I've rebased this and condensed it down to one commit. It's still failing 3 tests for me, and I'm not sure why. |
@MartyJRE are you able to complete this PR? i'd like to get the fix released. |
@ljharb seems I accidentally added a |
This reverts commit 7d659e7.
The reason for this is that the new feature is a breaking change and should be versioned as such.
Fixes #5.