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

MongooseArray#map(), #filter(), etc. should return vanilla JS arrays #8356

Closed
vkarpov15 opened this issue Nov 18, 2019 · 4 comments
Closed

Comments

@vkarpov15
Copy link
Collaborator

As opposed to Mongoose arrays. These methods create new arrays rather than modifying the existing array, and the newly created array isn't connected to a document. So we should consider making these methods return a vanilla JS array.

@vkarpov15 vkarpov15 added this to the Parking Lot milestone Nov 18, 2019
@vkarpov15
Copy link
Collaborator Author

See #8351

@AbdelrahmanHafez
Copy link
Collaborator

I am not familiar with all the uses of connecting an array to the parent document, but one way I would think about it is that:

If an array method returns entirely different values of the array items (such as map(...)), that should return a vanilla JS array, however on other methods such as filter(...) and slice(...), we are getting part of that mongoose array, which one could argue should be a mongoose array itself.

Is there more to it than just the type, functionality-wise?

@vkarpov15
Copy link
Collaborator Author

The only purpose of Mongoose arrays is to track changes and store them correctly in the parent document - if there isn't a parent document, there isn't much Mongoose arrays do that's useful. The only public functions I can find for mongoose arrays that aren't on vanilla JS arrays are addToSet(), nonAtomicPush(), set(), toObject(), and toBSON().

Maybe instead of returning a vanilla JS array, we should return an array that has set(), toObject(), toBSON(), but not the custom push(), pull(), etc methods that require an array parent.

@vkarpov15
Copy link
Collaborator Author

vkarpov15 commented May 10, 2020

After more careful consideration, I think @AbdelrahmanHafez is right in that you should be able to modify subdocuments like doc.arr.filter(doc => doc.createdAt < new Date('2019-01-01')).forEach(doc => { doc.archived = true; }). But we should still return vanilla arrays to make things more consistent: easier to explain that every array method returns a vanilla JavaScript array than say that some return vanilla arrays and some return Mongoose arrays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants