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

Removed null check for result array #20

Closed

Conversation

netsgnut
Copy link

This PR proposes removing a null check from the exec callback.

Currently when the filled values are not truthy (e.g. null or 0), it will not be filled into the virtual field, even when the default value is specified. There are cases where filling in a falsy value is part of the expected behavior, though.

Please advise if this is the correct approach to this. Thanks.

@wclr
Copy link
Owner

wclr commented Jun 12, 2018

Can you add test for this case?

@netsgnut
Copy link
Author

Thanks for the comment. After reading a bit more on the code, I think I might need a clarification as well, as it seems the problem that I had may stem from a misunderstanding.

The code that I tried to make it work is something as:

userSchema.fill('existentialCrisis').multi(function (users, ids, callback) {
  callback(null, ids.map(_id => null))
})

However it seems to work in some cases:

userSchema.fill('existentialCrisis').multi(function (users, ids, callback) {
  callback(null, ids.map(_id => ({ _id, existentialCrisis: null )))
})

I am not sure if the first code snippet (use of fill with multi only and ability to return falsey values) is a canonical use case that we should support, though.

@netsgnut
Copy link
Author

It seems I cannot reliably isolate a test case here. Let's close it for now? Thanks.

@netsgnut netsgnut closed this Jun 22, 2018
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.

2 participants