Skip to content

Commit

Permalink
Refine the way arguments are processed (closes #746, closes #767)(#765)
Browse files Browse the repository at this point in the history
- Convert number arguments into integer number using a custom `toInteger` implementation
- Change null/undefined/true/false handling strategy
- Review handling of arguments of a wrong type 
- Review usage of === undefined
  • Loading branch information
leshakoss authored and kossnocorp committed Jun 7, 2018
1 parent eae7d39 commit daddf06
Show file tree
Hide file tree
Showing 100 changed files with 451 additions and 102 deletions.
29 changes: 24 additions & 5 deletions CHANGELOG.md
Expand Up @@ -577,11 +577,29 @@ for the list of changes made since `v2.0.0-alpha.1`.
This change is introduced for consistency with ECMAScript standard library which does the same.
See [docs/Options.js](https://github.com/date-fns/date-fns/blob/master/docs/Options.js)

- **BREAKING**: all functions now handle arguments by following rules:

- **BREAKING**: all functions now implicitly convert arguments by following rules:

| | date | number | string | boolean |
|-----------|---------------|--------|-------------|---------|
| 0 | new Date(0) | 0 | '0' | false |
| '0' | Invalid Date | 0 | '0' | false |
| 1 | new Date(1) | 1 | '1' | true |
| '1' | Invalid Date | 1 | '1' | true |
| true | Invalid Date | NaN | 'true' | true |
| false | Invalid Date | NaN | 'false' | false |
| null | Invalid Date | NaN | 'null' | false |
| undefined | Invalid Date | NaN | 'undefined' | false |
| NaN | Invalid Date | NaN | 'NaN' | false |

Notes:
- as before, arguments expected to be `Date` are converted to `Date` using *date-fns'* `toDate` function;
- arguments expected to be numbers are converted to numbers using JavaScript's `Number` function;
- arguments expected to be strings arguments are converted to strings using JavaScript's `String` function.
- arguments expected to be numbers are converted to integer numbers using our custom `toInteger` implementation
(see [#765](https://github.com/date-fns/date-fns/pull/765));
- arguments expected to be strings arguments are converted to strings using JavaScript's `String` function;
- arguments expected to be booleans are converted to strings using JavaScript's `Boolean` function.

`null` and `undefined` passed to optional arguments (i.e. properties of `options` argument)
are ignored as if no argument was passed.

If any of resulting arguments is invalid (i.e. `NaN` for numbers and `Invalid Date` for dates),
an invalid value will be returned:
Expand All @@ -591,7 +609,8 @@ for the list of changes made since `v2.0.0-alpha.1`.
- `NaN` for functions that return numbers;
- and `String('Invalid Date')` for functions that return strings.

See tests and PR [#460](https://github.com/date-fns/date-fns/pull/460) for exact behavior.
See tests and PRs [#460](https://github.com/date-fns/date-fns/pull/460) and
[#765](https://github.com/date-fns/date-fns/pull/765) for exact behavior.

- **BREAKING**: all functions now check if the passed number of arguments is less
than the number of required arguments and throw `TypeError` exception if so.
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/addUTCMinutes/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -8,7 +9,7 @@ export default function addUTCMinutes (dirtyDate, dirtyAmount, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
date.setUTCMinutes(date.getUTCMinutes() + amount)
return date
}
5 changes: 5 additions & 0 deletions src/_lib/addUTCMinutes/test.js
Expand Up @@ -24,6 +24,11 @@ describe('addUTCMinutes', function () {
assert.deepEqual(result, new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 20)))
})

it('converts a fractional number to an integer', function () {
var result = addUTCMinutes(new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 0)), 30.5)
assert.deepEqual(result, new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 30)))
})

it('implicitly converts number arguments', function () {
var result = addUTCMinutes(new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 0)), '30')
assert.deepEqual(result, new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 30)))
Expand Down
9 changes: 5 additions & 4 deletions src/_lib/getUTCWeekYear/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'
import startOfUTCWeek from '../startOfUTCWeek/index.js'

Expand All @@ -17,13 +18,13 @@ export default function getUTCWeekYear (dirtyDate, dirtyOptions) {
locale.options &&
locale.options.firstWeekContainsDate
var defaultFirstWeekContainsDate =
localeFirstWeekContainsDate === undefined
localeFirstWeekContainsDate == null
? 1
: Number(localeFirstWeekContainsDate)
: toInteger(localeFirstWeekContainsDate)
var firstWeekContainsDate =
options.firstWeekContainsDate === undefined
options.firstWeekContainsDate == null
? defaultFirstWeekContainsDate
: Number(options.firstWeekContainsDate)
: toInteger(options.firstWeekContainsDate)

// Test if weekStartsOn is between 1 and 7 _and_ is not NaN
if (!(firstWeekContainsDate >= 1 && firstWeekContainsDate <= 7)) {
Expand Down
7 changes: 4 additions & 3 deletions src/_lib/setUTCDay/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -10,16 +11,16 @@ export default function setUTCDay (dirtyDate, dirtyDay, dirtyOptions) {
var options = dirtyOptions || {}
var locale = options.locale
var localeWeekStartsOn = locale && locale.options && locale.options.weekStartsOn
var defaultWeekStartsOn = localeWeekStartsOn === undefined ? 0 : Number(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn === undefined ? defaultWeekStartsOn : Number(options.weekStartsOn)
var defaultWeekStartsOn = localeWeekStartsOn == null ? 0 : toInteger(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn == null ? defaultWeekStartsOn : toInteger(options.weekStartsOn)

// Test if weekStartsOn is between 0 and 6 _and_ is not NaN
if (!(weekStartsOn >= 0 && weekStartsOn <= 6)) {
throw new RangeError('weekStartsOn must be between 0 and 6 inclusively')
}

var date = toDate(dirtyDate, dirtyOptions)
var day = Number(dirtyDay)
var day = toInteger(dirtyDay)

var currentDay = date.getUTCDay()

Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCDay/test.js
Expand Up @@ -86,6 +86,11 @@ describe('setUTCDay', function () {
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCDay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), 0.9)
assert.deepEqual(result, new Date(Date.UTC(2014, 7 /* Aug */, 31)))
})

it('implicitly converts number arguments', function () {
var result = setUTCDay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), '0')
assert.deepEqual(result, new Date(Date.UTC(2014, 7 /* Aug */, 31)))
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/setUTCISODay/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -7,7 +8,7 @@ export default function setUTCISODay (dirtyDate, dirtyDay, dirtyOptions) {
throw new TypeError('2 arguments required, but only ' + arguments.length + ' present')
}

var day = Number(dirtyDay)
var day = toInteger(dirtyDay)

if (day % 7 === 0) {
day = day - 7
Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCISODay/test.js
Expand Up @@ -49,6 +49,11 @@ describe('setUTCISODay', function () {
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCISODay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), 3.33)
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
})

it('implicitly converts number arguments', function () {
var result = setUTCISODay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), '3')
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/setUTCISOWeek/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'
import getUTCISOWeek from '../getUTCISOWeek/index.js'

Expand All @@ -9,7 +10,7 @@ export default function setUTCISOWeek (dirtyDate, dirtyISOWeek, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var isoWeek = Number(dirtyISOWeek)
var isoWeek = toInteger(dirtyISOWeek)
var diff = getUTCISOWeek(date, dirtyOptions) - isoWeek
date.setUTCDate(date.getUTCDate() - diff * 7)
return date
Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCISOWeek/test.js
Expand Up @@ -20,6 +20,11 @@ describe('setUTCISOWeek', function () {
assert.deepEqual(result, new Date(Date.UTC(2008, 11 /* Dec */, 31)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCISOWeek(new Date(Date.UTC(2004, 7 /* Aug */, 7)), 53.53)
assert.deepEqual(result, new Date(Date.UTC(2005, 0 /* Jan */, 1)))
})

it('implicitly converts number arguments', function () {
var result = setUTCISOWeek(new Date(Date.UTC(2004, 7 /* Aug */, 7)), '53')
assert.deepEqual(result, new Date(Date.UTC(2005, 0 /* Jan */, 1)))
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/setUTCWeek/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'
import getUTCWeek from '../getUTCWeek/index.js'

Expand All @@ -9,7 +10,7 @@ export default function setUTCWeek (dirtyDate, dirtyWeek, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var week = Number(dirtyWeek)
var week = toInteger(dirtyWeek)
var diff = getUTCWeek(date, dirtyOptions) - week
date.setUTCDate(date.getUTCDate() - diff * 7)
return date
Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCWeek/test.js
Expand Up @@ -20,6 +20,11 @@ describe('setUTCWeek', function () {
assert.deepEqual(result, new Date(Date.UTC(2008, 11 /* Dec */, 31)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCWeek(new Date(Date.UTC(2005, 0 /* Jan */, 2)), 1.1)
assert.deepEqual(result, new Date(Date.UTC(2004, 11 /* Dec */, 26)))
})

it('implicitly converts number arguments', function () {
var result = setUTCWeek(new Date(Date.UTC(2004, 7 /* Aug */, 7)), '53')
assert.deepEqual(result, new Date(Date.UTC(2005, 0 /* Jan */, 1)))
Expand Down
5 changes: 3 additions & 2 deletions src/_lib/startOfUTCWeek/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -10,8 +11,8 @@ export default function startOfUTCWeek (dirtyDate, dirtyOptions) {
var options = dirtyOptions || {}
var locale = options.locale
var localeWeekStartsOn = locale && locale.options && locale.options.weekStartsOn
var defaultWeekStartsOn = localeWeekStartsOn === undefined ? 0 : Number(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn === undefined ? defaultWeekStartsOn : Number(options.weekStartsOn)
var defaultWeekStartsOn = localeWeekStartsOn == null ? 0 : toInteger(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn == null ? defaultWeekStartsOn : toInteger(options.weekStartsOn)

// Test if weekStartsOn is between 0 and 6 _and_ is not NaN
if (!(weekStartsOn >= 0 && weekStartsOn <= 6)) {
Expand Down
9 changes: 5 additions & 4 deletions src/_lib/startOfUTCWeekYear/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import getUTCWeekYear from '../getUTCWeekYear/index.js'
import startOfUTCWeek from '../startOfUTCWeek/index.js'

Expand All @@ -14,13 +15,13 @@ export default function startOfUTCWeekYear (dirtyDate, dirtyOptions) {
locale.options &&
locale.options.firstWeekContainsDate
var defaultFirstWeekContainsDate =
localeFirstWeekContainsDate === undefined
localeFirstWeekContainsDate == null
? 1
: Number(localeFirstWeekContainsDate)
: toInteger(localeFirstWeekContainsDate)
var firstWeekContainsDate =
options.firstWeekContainsDate === undefined
options.firstWeekContainsDate == null
? defaultFirstWeekContainsDate
: Number(options.firstWeekContainsDate)
: toInteger(options.firstWeekContainsDate)

var year = getUTCWeekYear(dirtyDate, dirtyOptions)
var firstWeek = new Date(0)
Expand Down
13 changes: 13 additions & 0 deletions src/_lib/toInteger/index.js
@@ -0,0 +1,13 @@
export default function toInteger (dirtyNumber) {
if (dirtyNumber === null || dirtyNumber === true || dirtyNumber === false) {
return NaN
}

var number = Number(dirtyNumber)

if (isNaN(number)) {
return number
}

return number < 0 ? Math.ceil(number) : Math.floor(number)
}
64 changes: 64 additions & 0 deletions src/_lib/toInteger/test.js
@@ -0,0 +1,64 @@
// @flow
/* eslint-env mocha */

import assert from 'power-assert'
import toInteger from '.'

describe('toInteger', function () {
it('truncates positive numbers', function () {
var result = toInteger(10.99)
assert(result === 10)
})

it('truncates negative numbers', function () {
var result = toInteger(-5.5)
assert(result === -5)
})

it('converts convertable strings', function () {
var result = toInteger('-10.75')
assert(result === -10)
})

it('returns NaN for non-convertable strings', function () {
var result = toInteger('Foobar')
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for false', function () {
var result = toInteger(false)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for true', function () {
var result = toInteger(true)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for null', function () {
var result = toInteger(null)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for undefined', function () {
var result = toInteger(undefined)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for NaN', function () {
var result = toInteger(NaN)
assert(typeof result === 'number' && isNaN(result))
})

it('converts convertable objects', function () {
// eslint-disable-next-line no-new-wrappers
var result = toInteger(new Number(123))
assert(result === 123)
})

it('returns NaN for non-convertable objects', function () {
// eslint-disable-next-line no-new-wrappers
var result = toInteger({})
assert(typeof result === 'number' && isNaN(result))
})
})
3 changes: 2 additions & 1 deletion src/addDays/index.js
@@ -1,3 +1,4 @@
import toInteger from '../_lib/toInteger/index.js'
import toDate from '../toDate/index.js'

/**
Expand Down Expand Up @@ -27,7 +28,7 @@ export default function addDays (dirtyDate, dirtyAmount, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
date.setDate(date.getDate() + amount)
return date
}
5 changes: 5 additions & 0 deletions src/addDays/test.js
Expand Up @@ -20,6 +20,11 @@ describe('addDays', function () {
assert.deepEqual(result, new Date(2014, 8 /* Sep */, 11))
})

it('converts a fractional number to an integer', function () {
var result = addDays(new Date(2014, 8 /* Sep */, 1), 10.5)
assert.deepEqual(result, new Date(2014, 8 /* Sep */, 11))
})

it('implicitly converts number arguments', function () {
// $ExpectedMistake
var result = addDays(new Date(2014, 8 /* Sep */, 1), '10')
Expand Down
3 changes: 2 additions & 1 deletion src/addHours/index.js
@@ -1,3 +1,4 @@
import toInteger from '../_lib/toInteger/index.js'
import addMilliseconds from '../addMilliseconds/index.js'

var MILLISECONDS_IN_HOUR = 3600000
Expand Down Expand Up @@ -28,6 +29,6 @@ export default function addHours (dirtyDate, dirtyAmount, dirtyOptions) {
throw new TypeError('2 arguments required, but only ' + arguments.length + ' present')
}

var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
return addMilliseconds(dirtyDate, amount * MILLISECONDS_IN_HOUR, dirtyOptions)
}
5 changes: 5 additions & 0 deletions src/addHours/test.js
Expand Up @@ -24,6 +24,11 @@ describe('addHours', function () {
assert.deepEqual(result, new Date(2014, 6 /* Jul */, 12, 1, 0))
})

it('converts a fractional number to an integer', function () {
var result = addHours(new Date(2014, 6 /* Jul */, 10, 23, 0), 2.5)
assert.deepEqual(result, new Date(2014, 6 /* Jul */, 11, 1, 0))
})

it('implicitly converts number arguments', function () {
// $ExpectedMistake
var result = addHours(new Date(2014, 6 /* Jul */, 10, 23, 0), '2')
Expand Down
3 changes: 2 additions & 1 deletion src/addISOWeekYears/index.js
@@ -1,3 +1,4 @@
import toInteger from '../_lib/toInteger/index.js'
import getISOWeekYear from '../getISOWeekYear/index.js'
import setISOWeekYear from '../setISOWeekYear/index.js'

Expand Down Expand Up @@ -29,6 +30,6 @@ export default function addISOWeekYears (dirtyDate, dirtyAmount, dirtyOptions) {
throw new TypeError('2 arguments required, but only ' + arguments.length + ' present')
}

var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
return setISOWeekYear(dirtyDate, getISOWeekYear(dirtyDate, dirtyOptions) + amount, dirtyOptions)
}

0 comments on commit daddf06

Please sign in to comment.