timeout icon indicating copy to clipboard operation
timeout copied to clipboard

Remove convert timeout to number type in case typeof not string

Open spirinvladimir opened this issue 5 years ago • 2 comments

There are two tests which already cover this code base: 'should accept millisecond timeout' 'should accept string timeout' Both passed - ok.

spirinvladimir avatar Jun 10 '20 14:06 spirinvladimir

We may want to make a different change, perhaps actually guard against non-numbers if we are not going to convert inputs into numbers. This is because this change is altering the input behavior vs removing dead code -- and tests in the test suite are imperfect, they only test what someone put in there :) As an example, calling with the argument [2000] would set a 2s timeout before and after this change, but the .timeout property on the error will be an array instead of a number.

dougwilson avatar Aug 04 '20 18:08 dougwilson

According jsdoc for timeout function and current tests there are supported types:

  • number
  • string

Let look at example Number([2000, 2000]) should set delay to NaN. Other example Number(['2000']) should set delay to '2000' (as string). Only array with first element type number won't be supported after this PR. What is a reason to support Array instances here at all?

spirinvladimir avatar Aug 05 '20 11:08 spirinvladimir