arrays are polluted

[TrejGun]

Do you want to request a feature or report a bug?
bug

What is the current behavior?
unable to compare arrays using standard node's assert framework

(node:28580) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: ["test1"] deepStrictEqual [ 'test1' ]
    at run (~/test.js:35:10)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)
(node:28580) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:28580) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

If the current behavior is a bug, please provide the steps to reproduce.

#!/usr/bin/env node
"use strict";

const assert = require("assert");
const mongoose = require("mongoose");
mongoose.set("useFindAndModify", false);
const {Schema, connection} = mongoose;
const DB = "6880";
const URI = `mongodb://localhost:27017/${DB}`;
const OPTS = {family: 4, useNewUrlParser: true};

function arrayGetter(array) {
  return [...array];
}

const schema = new Schema({
  array: {
    type: [String],
    // get: arrayGetter, // uncomment me to fix issue
  },
});

const Test = mongoose.model("test", schema);

const array = ["test1"];

const test = new Test({array});

async function run() {
  await mongoose.connect(URI, OPTS);
  await connection.dropDatabase();
  const doc = await Test.create(test);
  assert.ok(doc);
  const model = await Test.findById(doc._id);
  assert.deepStrictEqual(model.array, array);
  console.log("All Assertions Pass.");
  await connection.close();
}

run();

What is the expected behavior?
model.array should be pure array, no additional hidden fields

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

node: 8.15.1
mongodb 4.0.6
mongoose: 5.0.0 - 5.4.20

[freewil]

use toObject()

assert.deepStrictEqual(model.array.toObject(), array);

[ubald]

@freewil I'm having the same kind of issues comparing CoreMongooseArrays with Arrays. But you can't always use .toObject(). Using a sinon spy's calledWith won't let you toObject() the internal args to the function call.

Currently upgrading mongoose on a relatively large project and I have tests failing by the dozen because of that

[TrejGun]

@freewil in thsi case getter is a better workaround

[vkarpov15]

@TrejGun your custom getter is the best way to work around this. assert.deepStrictEqual() will fail if any non-symbol array properties are different and if you're using a subclassed array:

$ node
> const assert = require('assert')
undefined
> class MyArr extends Array {}
undefined
> assert.deepStrictEqual(new MyArr(), new Array())
AssertionError [ERR_ASSERTION]: MyArr [] deepStrictEqual []
> const arr1 = []
undefined
> const arr2 = []
undefined
> assert.deepStrictEqual(arr1, arr2)
undefined
> arr1.foo = 'bar'
'bar'
> assert.deepStrictEqual(arr1, arr2)
AssertionError [ERR_ASSERTION]: [ foo: 'bar' ] deepStrictEqual []

The only realistic possibility would be to support something like this:

assert.deepStrictEqual(model.array, new mongoose.Types.Array(array));

Which we can do, but will require some substantial work. Would this work for your use case @TrejGun @ubald ?

[TrejGun]

You can add this getter when compliling model

something like

ret = localFieldPath.applyGetters(doc[localField], [...value]);

[vkarpov15]

@TrejGun that's one way to do it, but then you always get arrays as plain arrays, so no Mongoose change tracking on .push(), etc.

[TrejGun]

oh i see
is there a way to separate

const schema = new Schema({
  array: {
    type: [String],
  },
});

from

const schema = new Schema({
  array: {
    type: new MongooseArray(String),
  },
});

or its better to use lean and arrayGetter in tests?

[vkarpov15]

@TrejGun lean() is one option. I generally just use .toObject() before doing any deep equality checks, like this.

[vkarpov15]

Almost done, still need to switch _atomics to a symbol. Will wait for the next release though to avoid changing too much at one time.

[vkarpov15]

Fix will be in 5.5.6. The answer is to do this:

assert.deepStrictEqual(model.array, new mongoose.Types.Array(array));

Because two objects cannot be deepStrictEqual() unless they are of the same type.

[TrejGun]

@vkarpov15 good to know, thanks

[vkarpov15]

Need to reopen this because it looks like this no longer works in Node.js 12: #7784

[vkarpov15]

I opened up an issue with Node core about this: nodejs/node#27652 . We'll see what they come up with.

[BridgeAR]

@vkarpov15

that's one way to do it, but then you always get arrays as plain arrays, so no Mongoose change tracking on .push(), etc.

This is IMO an ideal use case for proxies. That way it's possible to pretend to be a clean array while still being able to track changes.

I opened up an issue with Node core about this: nodejs/node#27652 . We'll see what they come up with.

The issue is only about loose equal comparison. If I'm not mistaken your fix is to use a symbol properties instead of string properties. That will still fail using the strict assertion. The loose assertion is actually deprecated and I recommend not to use it.

assert.deepStrictEqual(model.array, new mongoose.Types.Array(array));

This would of course also be a possibility while it moves the complexity to the user to be aware that the returned type is not just an array. Using the proxy as proposed above would allow to hide all the internals so that the users do not have to worry about this anymore.

[vkarpov15]

Thanks for the feedback. I'm aware that deepEqual() is deprecated and this approach only works with deepEqual() as opposed to deepStrictEqual(). We've been thinking about using proxies for arrays for quite a while, it's something we'll experiment with more going forward.

Refactoring to use symbols was beneficial for internal readability, because so many objects in Mongoose have _parent properties, but having deepEqual() work was a nice side benefit.

[TrejGun]

wow