Skip to content

Conversation

yaodingyd
Copy link
Contributor

Current parse would return all value as string but I would like an option to return number as number.

@zepod
Copy link
Contributor

zepod commented Mar 18, 2019

I may be worth it to update typescript types

@sindresorhus
Copy link
Owner

I don't think number is clear enough as an option name. It doesn't describe what it does. Need a more explicit name.

@yaodingyd
Copy link
Contributor Author

How about parseAsNumber?

@Andrew-Lahikainen
Copy link

Andrew-Lahikainen commented Apr 30, 2019

Would it make sense to make this option a little more generic so it can handle other primitives? Or do you think there is a possibility that someone might want number conversions and not boolean conversions for example?

If you want to have a generic name I think something like parseTypes or parsePrimitiveTypes.

If you want to handle each primitive separately, I think prepending parse might be acceptable?

E.g.:

  • parseNumbers
  • parseBooleans
  • parseNulls
  • parseUndefineds
  • parseSymbols (this one probably doesn't make sense to implement)

@sindresorhus
Copy link
Owner

Would it make sense to make this option a little more generic so it can handle other primitives?

I don't see us adding more than number and boolean parsing. It can quickly get ambiguous. I would have preferred to have them under one option, but I have feeling people have use-cases for only either.

@sindresorhus
Copy link
Owner

I prefer the parseNumber option name.

@Andrew-Lahikainen
Copy link

I prefer plural since this option parses all numbers, not just one. But that's being really nitpicky :p

@sindresorhus
Copy link
Owner

Alright, parseNumbers.

@HHK1
Copy link

HHK1 commented May 31, 2019

Thanks for this option, it's awesome !
Are things missing before it can be merged ? Just to have an idea of when it will included in a release :)

@sindresorhus sindresorhus changed the title add number option to return value as number Add parseNumbers option to .parse() Jun 5, 2019
@sindresorhus
Copy link
Owner

Pretty close to being done. The merge conflict needs to be resolved.

@sindresorhus sindresorhus merged commit f7be831 into sindresorhus:master Jun 8, 2019
@sindresorhus
Copy link
Owner

Looks good. Thanks :)

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.

5 participants