d3 / d3-dsv

A parser and formatter for delimiter-separated values, such as CSV and TSV.
https://d3js.org/d3-dsv
ISC License
436 stars 76 forks source link

Incorrect decoding of local dates in Safari. #53

Closed Fil closed 5 years ago

Fil commented 5 years ago

instead of treating new Date("2018-04-25T11:00") as a local date, Safari parses it as a UTC date and returns it with the tz difference added. This fix detects the situation by checking TWO local dates (one in winter, one in summer) which should return hour 0, and if one of them is not 0 it's the Safari bug. In that case, and if the date string that is passed is local (no "Z"), we shift it accordingly.

I don't know how to write a node test for this, but the following notebook shows the fix on Safari: https://observablehq.com/d/7e8b78e5050258ae

Fixes https://github.com/d3/d3-dsv/issues/45

mbostock commented 5 years ago

Some suggestions:

if (fixtz && !m[7]) value += fixtz;
value = new Date(value)
mbostock commented 5 years ago

Oh, and most importantly: don’t add the local timezone offset if the specified input value doesn’t contain a time (T…, m[4] should be truthy, I believe).

if (fixtz && !!m[4] && !m[7]) value += fixtz;
value = new Date(value)

In other words, this should always be parsed as UTC (and should already work as-is in all browsers):

new Date("2018-04-25")

And this would give an invalid date:

new Date("2018-04-25-07:00")

Let’s make sure we have a test for that.

Fil commented 5 years ago

I've implemented the suggested changes, but I realize the fix is wrong in winter for summer dates, and vice versa. (Because the offset depends on the daylight savings time at the date we read.) So, more work needed…

mbostock commented 5 years ago

Okay, well here’s potentially a simpler hack to force Safari to parse local dates. Change the format:

new Date("2019/09/02 11:00")

Something like value.replace(/-/g, "/").replace(/T/, " ") should work.

Fil commented 5 years ago

Much simpler! I've updated the test notebook. I still have no idea how to test for it in node (can we monkey patch the Date class?).