search

nextstrain / nextstrain.org

The Nextstrain website
https://nextstrain.org
GNU Affero General Public License v3.0
87 stars 49 forks source link

List Resources: re-evaluate date parsing calculation #893

Open victorlin opened 3 weeks ago

victorlin commented 3 weeks ago

https://github.com/nextstrain/nextstrain.org/blob/aae874c06cd6d17616710c501ad73104a7c5e051/static-site/src/components/ListResources/Modal.tsx#L134-L146

from @ivan-aksamentov in https://github.com/nextstrain/nextstrain.org/pull/874#discussion_r1626869069:

Another thing is that this entire calculation is wrong. For example, not all years have 365 days. Date-time calculations is a very hard topic, and it is somewhat unreasonable to try and do on the spot. There are specialized high-quality libraries exist to do this correctly, e.g. luxon (continuation of moment.js). And then for human durations there are also small libs (e.g. this). Not even talking about that calendars and durations are different in different cultures.

tsibley commented 3 weeks ago

+1 for using fully-fledged datetime libraries. For JS/TS, that typically means Luxon. (Having some déjà vu here.)

ivan-aksamentov commented 3 weeks ago

In this particular case, James convinced me in the linked thread that because the output is for human consumption and vague/approximate by design, then it does not necessarily make sense to spend energy on being pedantic here.

There are many examples of reinventing the wheel in the org's code. And it kinda works. So the team probably has better things to do.

(As of my general preferences, I believe if a precise, typed one-liner requiring no maintenance is available on npm, then it should be prioritized, at least when writing new code)