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

When setting nested array paths, look for object paths not just array paths #2

Closed
wants to merge 1 commit into from

Conversation

clayne11
Copy link
Collaborator

@clayne11 clayne11 commented Oct 7, 2016

Fixes #1

@clayne11 clayne11 force-pushed the set-object-paths branch 3 times, most recently from 2fdad34 to 542a680 Compare October 7, 2016 02:49
…st array paths

When dealing with a plain object and not a Mongo modifier we want to
traverse object paths and deeply set values.

Fixes #1
@aldeed
Copy link
Collaborator

aldeed commented Dec 26, 2016

@clayne11 Thanks for the PR. It's possible there is something to fix here, but a position string is never supposed to have any periods in it unless they are part of the object key. So this fix doesn't seem correct.

It's done this way to support Mongo modifier objects. For example, {$set: {'a.b.c': 1}} would have a position string like $set[a.b.c]. So we never want to split on periods because they are part of the key. By contrast, the position key for {$set: {a: {b: {c: 1}}}} should be $set[a][b][c]. It's all done in bracket notation.

I might be misunderstanding what you're doing here, or maybe there is a different solution for the core problem this is fixing.

@clayne11
Copy link
Collaborator Author

clayne11 commented Jan 2, 2017

The issue is that this library supports both Mongo modifier objects and plain JS objects. The fix I put in only runs on plain JS objects, not on Mongo modifier objects. The issue occurred when trying to set an array value in a plain JS object that was nested under an object key. You can see the tests I put it in that show the issue. If you run those tests without the changes I made to the code you can see how it resolved the issue.

@clayne11 clayne11 closed this Mar 30, 2020
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.

Setting nested array values does not work properly
2 participants