vvo / tzdb

🕰 Simplified, grouped and always up to date list of time zones, with major cities
MIT License
772 stars 53 forks source link

Remove Luxon validation, so the script can deliver proper results in any supported Node.js version. #271

Open chekrd opened 1 year ago

chekrd commented 1 year ago

Hi @vvo, thanks for this package, it helped us a lot.

This is more of a discussion than an issue. It is a follow up for already fixed issue https://github.com/vvo/tzdb/issues/267.

I was part of the investigation in cooperation with NBaruK.

I would like to know the reason behind the following code (link to the module):

const tz = DateTime.fromObject(
  {},
  {
    zone: timeZoneName,
  },
);

if (tz.isValid !== true) {
  console.error(
    "Time zone data not accurate, please investigate",
    tz.invalidReason,
    timeZoneName,
  );
  continue;
}

DateTime.fromObject comes from Luxon package. Then, inside Luxon, the validation of the input is done using Intl API (link to the module within Luxon).

This is the reason why platform version (Node.js) affects the ouput. And I can't understand the real meaning of the timezone validation in generate.js. Timezones input is from IANA - I would say we can trust it as it is a source for Intl used by Node.js, browsers, etc.). So you validate the timezone against the same database, but if the current installed platform (nodejs) is not up to date with the IANA DB, it fails. It does not mean the input is wrong.

I would be glad if you can elaborate on the code. Is it necessary? Can you remove this validation from your package?

vvo commented 1 year ago

This was 2 years ago, and I do not remember why this was added. Probably because I use these time zone names on the backend/frontend to generate dates myself using Luxon in a project. So if the timezone names doesn't pass Luxon's validation it would fail for me too.

Is this currently causing any trouble? Thanks!

chekrd commented 1 year ago

As long as you are using IANA (that's what tzdb is about), Luxon validation in tzdb is useless as I described in the first post. The only thing Luxon validation can do is remove valid input timezones based on the Node.js version, thus silently breaking a final app. I guess you didn't know you may stopped supporting Kiev timezone in your projects out of a sudden.

Is this currently causing any trouble?

Currently no. But the source of the issue is not fixed, so we can expect regression anytime.

That's why I think Luxon validation should be removed.