search

Stigmatoz / web-activity-time-tracker

Chrome Extension that tracks and limits time you spent on sites
https://chrome.google.com/webstore/detail/web-activity-time-tracker/hhfnghjdeddcfegfekjeihfmbjenlomm
MIT License
757 stars 115 forks source link

Remove locale from toLocaleDateString. #51

Closed tschettler closed 3 years ago

tschettler commented 3 years ago

Fixes #45, #46

Locale usage is inconsistent, removing the explicitly passed locale parameter will use the default locale:

In basic use without specifying a locale, a formatted string in the default locale and with default options is returned.

-- Source: MDN

Stigmatoz commented 3 years ago

I cannot accept the pull request. These are good changes, it is a bad idea to use the en-US locale. But these changes will break all previous data in the storage. It might be worth changing the tolocaleDateString() date after loading the data.

tschettler commented 3 years ago

Thanks for the feedback, good idea. I see your change, do you think it'd be even better to use an ISO date for storage instead?

Stigmatoz commented 3 years ago

Initially, I chose the wrong option, use toLocaleDateString ('en-US') everywhere. And now, if we change the date and time format, the storage will break for users whose format is different from en-US.

tschettler commented 3 years ago

Yep, I now completely understand the issue. However, since you are using "en-US" now, it would be straightforward to convert all existing data to the YYYY-MM-DD ISO 8601 format and move completely away from specified locales for date storage.