search

ljharb / js-traverse

MIT License
45 stars 8 forks source link

Revert "[New] support enumerable Symbol properties" #6

Closed MartyJRE closed 4 months ago

MartyJRE commented 5 months ago

This reverts commit 7d659e78d43e69e6afc604e12b3ba3a5def0e46f.

The reason for this is that the new feature is a breaking change and should be versioned as such.

Fixes #5.

MartyJRE commented 5 months ago

@ljharb I added options to traverse. It now only deals with string keys by default. When given includeSymbols: true inside the options, it functions the way you desgined.

Also added immutable to options to avoid having too many parameters.

ljharb commented 5 months ago

(you'll also need to rebase; i pulled out the immutable → options refactor)

MartyJRE commented 5 months ago

@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.

MartyJRE commented 5 months ago

The Automatic Rebase / _ / Automatic Rebase (pull_request_target) job still seems to think there are conflicts. However, when I try to merge locally, there are no conflicts to be seen.

ljharb commented 5 months ago

Don’t worry about the automatic rebase workflow; it’s failing because you made your PR from “main”.

ljharb commented 5 months ago

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.

ljharb commented 4 months ago

@MartyJRE are you able to complete this PR? i'd like to get the fix released.

MartyJRE commented 4 months ago

@ljharb seems I accidentally added a ! in one condition, the change should be good to go now