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

fix(Decimal128): throw error when passing a string to Decimal128 constructor #322

Closed
wants to merge 1 commit into from
Closed

Conversation

vkarpov15
Copy link
Contributor

You can get some nasty surprises when you pass a string to the Decimal128 constructor:

const bson = require('bson');

const v = new bson.Decimal128('14.92');

// {"$numberDecimal":"8.740930561E-6167"}
console.log(JSON.stringify(v));

Would be handy to at least throw an error. Although is there some reason why we can't just do fromString() if the user passes a string instead of a buffer to the Decimal128 constructor?

@vkarpov15
Copy link
Contributor Author

Re: Automattic/mongoose#7815

@mbroadst
Copy link
Member

mbroadst commented Jun 7, 2019

I can't think of a reason we wouldn't support a string in the constructor, other than nobody thought to do it when they were originally implementing the module. How about adding that here with a test?

@daprahamian
Copy link
Contributor

I'm on board with

function Decimal128(bytes) {
  if (typeof bytes === 'string') {
    return Decimal128.fromString(bytes);
  }
  this.bytes = bytes;
}

@agolin95 agolin95 added the tracked-in-jira There is a ticket in Mongo's Jira instance tracking this issue/PR label Apr 8, 2021
@agolin95
Copy link
Contributor

agolin95 commented Apr 8, 2021

@bajanam
Copy link

bajanam commented Mar 7, 2022

Thank you for submitting this PR! While our current development plan is at capacity, we will track this in Jira for future prioritization.

@bajanam bajanam closed this Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracked-in-jira There is a ticket in Mongo's Jira instance tracking this issue/PR
Projects
None yet
5 participants