-
-
Notifications
You must be signed in to change notification settings - Fork 364
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
Make saveUnknown work recursively #522
Conversation
Pull Request Test Coverage Report for Build 818
💛 - Coveralls |
Pull Request Test Coverage Report for Build 823
💛 - Coveralls |
I would also like to add an option to |
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.
Looks good at first glance. We for sure need tests for this. I'd also like to see some more comments explaining a bit more what exactly this is doing, just to make it easier for future devs to work on it.
I'm all for that, so long as the default stays as the current behavior (no breaking change). It also probably makes sense to have that be a separate PR. |
Co-Authored-By: dobrynin <David.Dobrynin@gmail.com>
@fishcharlie, I've added your requested changes. As for making a new PR for |
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.
LGTM
Summary:
When fetching from DynamoDB with
saveUnkown === true
, nested fields do not get returned. This PR aims to fix that.GitHub linked issue:
Closes #323
Type (select 1):
Is this a breaking change? (select 1):
Is this ready to be merged into Dynamoose? (select 1):
Are all the tests currently passing on this PR? (select 1):
Other:
npm test
from the root of the project directory to ensure all tests continue to pass