Skip to content

Fix 604 #605

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

Merged
merged 2 commits into from
Feb 22, 2023
Merged

Fix 604 #605

merged 2 commits into from
Feb 22, 2023

Conversation

mcollina
Copy link
Member

Fixes #604

Checklist

Copy link
Member

@ivan-tymoshenko ivan-tymoshenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit cb736a2 into master Feb 22, 2023
@mcollina mcollina deleted the fix-604 branch February 22, 2023 11:58
throw new Error(`The value "${i}" cannot be converted to a number.`)
} else if (!Number.isFinite(num)) {
Copy link
Member

@ivan-tymoshenko ivan-tymoshenko Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Uzlopak I will approve it because I'm not sure if it makes any perf difference with i === Infinity || i === -Infinity, but if it does, feel free to open a new PR.

@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 22, 2023

It would have been this:

asNumber (i) {
    const num = i
    if (typeof i !== 'number') {
      i = Number(i)
    }
    // NaN !== NaN
    if (i !== i) { // eslint-disable-line no-self-compare
      throw new Error(`The value "${num}" cannot be converted to a number.`)
    }
    if (i === Infinity || i === -Infinity) {
      return 'null'
    }
    return '' + i
  }

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

Successfully merging this pull request may close these issues.

Broken error message for missing required property
3 participants