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

Square bracket syntax support #29

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aarondancer
Copy link

@aarondancer aarondancer commented May 14, 2019

I realize that this was already attempted in #24, but with a little extra work I was able to support the edge cases mentioned in the thread.

I tested these changes against the lodash.get tests and it's mostly compatible. The lodash.get tests failing all seem like uncommon edge-cases, so I've opted not to address them.

The following tests from lodash.get fail:

`dlv` should preserve the sign of `0`
`dlv` should get symbol keyed property values
`dlv` should get a key over a path
`dlv` should not ignore empty brackets
`dlv` should handle empty paths
`dlv` should handle complex paths
`dlv` should follow `path` over non-plain objects
`dlv` should return the default value when `path` is empty

This will also address #27

cc @developit @bwendt-mylo

@aarondancer aarondancer changed the title Array support Square bracket syntax support May 15, 2019
@developit developit self-requested a review May 22, 2019 12:56
@developit
Copy link
Owner

Hey @aarondancer! Thanks for opening the PR. Would you be able to provide some context on why square bracket notation is important for your use-case?

@maraisr
Copy link

maraisr commented May 23, 2019

I dare say it make's composing array like indices easier to reason about. Like "im indexing an array, as opposed to accessing a property"... but imo you may very just go dlv(obj, 'a.b.0.c'); dlv(obj2, 'a.b.1.c');, which keep the bundle size down too, and avoids regex.

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

3 participants