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

Can we try object.toString before Object.prototype.toString? #78

Closed
arasmussen opened this issue Sep 17, 2023 · 6 comments
Closed

Can we try object.toString before Object.prototype.toString? #78

arasmussen opened this issue Sep 17, 2023 · 6 comments

Comments

@arasmussen
Copy link
Contributor

Running into the following issue when comparing MongoDB ObjectIds:

import mongoose from 'mongoose';

const foo = mongoose.Types.ObjectId(123);
const bar = mongoose.Types.ObjectId(456);

Object.prototype.toString(foo); // '[Object object]'
foo.toString(); // '000001c81ce7420101e4238e'

Object.keys(foo); // []

isEqual(foo, bar); // true

npm versions:

mongoose: ^6.12.0
mongodb: ^4.17.0
expect: ^1.20.2

expect uses is-equal: ^1.5.1, so pushing 1.6.5 would resolve this issue. Happy to submit a PR! Just want to know if it'd be approved.

@arasmussen
Copy link
Contributor Author

arasmussen commented Sep 18, 2023

To be clear, my proposal is:

https://github.com/inspect-js/is-equal/blob/main/why.js#L46-L50

If value.toString exists and is a function, and other.toString exists and is a function, call and compare the results. If they differ, specify a why message (that the toString results are different).

It seems like this would solve problems for any objects that define their own toString serialization method.

@ljharb
Copy link
Member

ljharb commented Sep 18, 2023

That’s not a bad comparison to add; we could check valueOf and Symbol.toPrimitive as well, and use es-to-primitive with different hints to cover them all.

That way we don’t have to check specific methods, we can just compare “coercing to a string” and “coercing to a number”, altho we’d have to try/catch to account for things that throw (like symbols), in which case “both throw” would be considered equal.

@arasmussen
Copy link
Contributor Author

@ljharb submitted two PRs, one with the fix, and another for the version bump. Used es-to-primitive with separate hints, as you requested. Lmk if you have feedback.

@ljharb
Copy link
Member

ljharb commented Sep 27, 2023

Thanks, I’ll take a look - versions are never bumped in PRs, though, nor by non-maintainers, anywhere on github.

@arasmussen
Copy link
Contributor Author

Thanks for merging! Sorry to keep bugging you – do you have an ETA on 1.6.5? Eager to get this updated in our project.

@ljharb
Copy link
Member

ljharb commented Oct 7, 2023

It’d be 1.7.0, and not just yet, but hopefully soon.

Fixed by #79.

@ljharb ljharb closed this as completed Oct 7, 2023
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