MikeMcl / decimal.js

An arbitrary-precision Decimal type for JavaScript
http://mikemcl.github.io/decimal.js
MIT License
6.45k stars 475 forks source link

Prototype assignment (replacement) #161

Closed KACAH closed 3 years ago

KACAH commented 4 years ago

Hey!

Right now Decimal prototype is replaced by completely new object P. According to MDN article about inheritance and prototypes this is considered a bad practice:

do not set the prototype f.prototype = {b:3,c:4}; this will break the prototype chain

The consequence is - some libraries can't identitfy Decimal instances as custom JS objects and break them trying to clone, serialize, etc. The example of such library is MobX v6. which is checking "plain" objects like this.

const plainObjectString = Object.toString()

export function isPlainObject(value) {
    if (!isObject(value)) return false
    const proto = Object.getPrototypeOf(value)
    if (proto == null) return true
    return proto.constructor?.toString() === plainObjectString
}

The result for Decimal instances is 'true', which is incorrect.

Is it possible to replace prototype assignment with prototype properties assignment?

For instance replace

Decimal.prototype = P;

with

for (var protoKey in P) {
  if (Object.prototype.hasOwnProperty.call(P, protoKey)) {
    Decimal.prototype[protoKey] = P[protoKey];
  }
}
MikeMcl commented 4 years ago

Here the constructor property is set on the instance not the prototype because the prototype may be shared between multiple Decimal constructors, so the value of the constructor property may be different for different instances.

The issue with the isPlainObject function returning true could be solved by adding the following after the shared prototype object is assigned to Decimal.prototype.

Decimal.prototype.constructor = Decimal;

I have not included this previously because Decimal.prototype.constructor is shadowed by instance.constructor, but yes, it would solve this issue so I'll consider adding it to the next release.

Thanks for your input.

KACAH commented 4 years ago

Thank you!

MikeMcl commented 3 years ago

Fixed in v10.3.0.