search

visionmedia / bytes.js

node byte string parser
MIT License
458 stars 57 forks source link

Too relaxed value validator? #31

Closed ffigiel closed 8 years ago

ffigiel commented 8 years ago

I just noticed this behavior:

> bytes('250Kg');
250
> bytes('250 e-mails');
250

Is this a bug? I would expect a NaN instead of 250B, like in this example:

> bytes('something 250');
NaN
dougwilson commented 8 years ago

Hi @megapctr, thanks for the report! It does look like an oversight in the parsing. I would actually go as far as say I would except both of those examples to throw an error, rather than simply return NaN, since the purpose is to parse, and that would be a parse error. Otherwise, returning null would be in line with the other inputs you can give.

@theofidry do you have any thoughts on this?

theofidry commented 8 years ago

Will look into it tonight :)

ffigiel commented 8 years ago

Thanks for super quick reaction to my issue! :zap: I see that bytes@2.3.0 doesn't include this fix, what's the eta for 2.4.0?

theofidry commented 8 years ago

@megapctr we cannot ship it for 2.x as it brings breaking changes (you can no longer use bytes('2 bytes') for example. So it will be for the next major release.

@dougwilson do you have any release date in mind?

dougwilson commented 8 years ago

@theofidry release for 3.0 is just as soon as we feel it's ready to go :) I did find a couple things I wanted to fix, and should make issues here in this repo to tack (and for anyone else to make a PR for before I can).

theofidry commented 8 years ago

:+1: