Skip to content
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

Throw an error when I18n.strftime() takes an invalid date #383

Merged
merged 1 commit into from
Jan 6, 2016
Merged

Throw an error when I18n.strftime() takes an invalid date #383

merged 1 commit into from
Jan 6, 2016

Conversation

shinnn
Copy link

@shinnn shinnn commented Jan 6, 2016

Problem

JavaScript Date instance can be an invalid date in some cases. For example,

// When `Date` constructor takes a valid date string
new Date('20001010'); //=> Tue Oct 10 2000 09:00:00 GMT+0900 (JST)

// When `Date` constructor takes an **invalid** date string
new Date('200010100'); //=> Invalid Date

Currently, I18n.strftime produces a wrongly formatted string when it takes an invalid date object. For example,

I18n.strftime(new Date('foo'), '%Y%m%d');
//=> 'NaNaNaN'

Solution

I updated I18n.strftime to immediately throw an error when it takes an invalid date object. This change prevents it from producing broken strings like NaNaN.

@@ -772,6 +772,10 @@

options = this.prepareOptions(options, DATE);

if (isNaN(date.getTime())) {
throw new Error('I18n.strftime() requires a valid date object, but received an invalid date.');

Choose a reason for hiding this comment

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

Does Error work in every JS environment?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, Error constructor is one of the global objects that all the JS environments (ancient browsers, Node.js, Electron etc.) have.

@PikachuEXE
Copy link

Can you add an entry to CHANGELOG?
Then I will merge it.

Currently, I18n.strftime produces a wrongly formatted string when it takes an invalid date object. For example,

```
I18n.strftime(new Date('foo'), '%Y%m%d');
//=> 'NaNaNaN'
```
@shinnn
Copy link
Author

shinnn commented Jan 6, 2016

Can you add an entry to CHANGELOG?

f0d4d31#diff-4ac32a78649ca5bdd8e0ba38b7006a1eL17

@PikachuEXE
Copy link

Thanks!
Merging

PikachuEXE added a commit that referenced this pull request Jan 6, 2016
Throw an error when I18n.strftime() takes an invalid date
@PikachuEXE PikachuEXE merged commit 264aba7 into fnando:master Jan 6, 2016
@shinnn
Copy link
Author

shinnn commented Jan 6, 2016

No problem! Thanks for your speedy review :)

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.

2 participants