I'd love to include a test, but this is really finicky to reproduce, unfortunately; you need to be in a place that has DST, then compare a date with DST and one without. Even if there is a way to set or fake the local time and zone, that would probably be global and break all other tests.

Hi. I was running into a similar issue when using this package to parse csv files. Now that the UK is back to GMT, dates that were in BST (GMT 1) are being parsed as the day before because of the difference in UTC offset.

I've noticed that there are a couple of failing tests as a result of the changes in this PR:
roundtrip features should preserve dates [n] -> (d) -> [d] -> (n):

With regards to adding tests to replicate the scenario for this fix, have you had a look at the timekeeper npm module? It allows you to freeze time at a certain date, and then unset time again after your tests run. However, I have noticed that the xlsx package needs to be imported after the time has been frozen otherwise it will not work.

Hi @lalomartins

I have fixed the failing units that I mentioned in my previous post, but I don't have permissions to push them to GrabCAD:master or to a branch.

It would be great if you could give me temporary permissions to push either to the GrabCAD:master, or to a branch where I could create a pull request.

The reason that the tests were failing was that, one of the existing tests uses the datenum() function, which you changed as part of your pull request, but as the datenum() function is not exposed as part of the xlsx api, someone had copy and pasted the old version into the test file, and this just needed to be updated to your new version.

This doesn't seem very nice, or maintainable going forward, but I'm just working with what's there.

Well, thanks for pitching in to that!

I guess the normal github way to do it would be to fork and make a PR, right? I actually can't give you permissions on GrabCAD repo since it belongs to the company. You can do the “compare between forks” thing when creating the PR.

If you don't want to bother with all that work you can also produce a patch file and put it somewhere I can get to.

@lalomartins No probs. I've submitted a new PR for this, with the fixed unit tests.

I merged your changes as well, so now the two PRs should be equivalent.

@lalomartins Great. I've closed my version to avoid confusion.

@SheetJSDev Could I know when this fix will be merged and make available?

Unless im mistaken, this would also fix #874.

Any news with this issue? i think should be merged

Is the PR ready to merge? We would need that fix in production. Thx!

This should be merged ASAP :) Thanks !

Is there an ETA on this merge?

Please can someone supply an ETA on this merge?

We're also running into this problem. FWIW, we're using the latest Pro version. Please merge this @SheetJSDev

Thanks a lot @lalomartins and apologies for sitting on this, we've been hesitant to touch the date logic but finally decided to do it as part of the 0.16 release

What's the ETA on 0.16? @SheetJSDev

Did this get merged in? I am seeing this issue on latest (0.16.2)

It's merged in the Pro version at least. For the Pro version, you have to specify XLSX.set_date_style(1); to get it to work.

@sahilsaid this was merged in and the default was changed in this build. In our Pro builds, the old date style is still the default, but you can change it using XLSX.set_date_style as mentioned by @Ugoku

Thanks @Ugoku and @SheetJSDev
I am using the community version. Any idea when this fix will be available for community version?

so this really is only supported in pro version?? i'd rather implement the import with a different library then.