search

moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.33k stars 730 forks source link

Changing Defaults (setZone) #1487

Open sscowden opened 1 year ago

sscowden commented 1 year ago

Is your feature request related to a problem? Please describe. We use a code generator (NSwag) to generate client code for our client apps in Typescript. It generates DateTime.fromISO([date value as string]), which will by default not include the offset from the string without using setZone: true

Describe the solution you'd like I'd like to be able to globally default setZone to true anytime DateTime.formISO is used. Or more broad stroke, would be great to be able to change many of the default values used in Luxon for some

Describe alternatives you've considered

Additional context I've reviewed docs and don't see a way to do this, but if I've missed something please let me know. Thanks!

icambron commented 1 year ago

I guess I don't see why you create a thin wrapper and generate that instead?

const myFromIso = (s: string) => DateTime.fromISO(s, { setZone: true })

The reason we don't make this a global setting is that it's usually highly inadvisable, and we don't want to make footguns easier to use. What usually happens is that code ends up assuming that it operates in a certain zone, and you don't usually control the strings. For example:

const dt = DateTime.fromISO(someStringIGotFromAnAPI)

// check something, like if it's between 8 and 12
if (dt.hour >= 8 && dt.hour < 12) {
   doSomeImportantThing()
}

That code always does what you expect, because 8 and 12 are in the zone you expect them to be, namely the zone you're environment running in, or some zone you've set explicitly. But if you use setZone: true for everything (and especially if you use it implicitly because of a setting we introduce), then one day some string will be specified in some other zone and the 8 and 12 will suddenly mean completely different times, and thus result in completely different program behavior, all based on how some caller happened to format their ISO string. Making things worse is that the string specifies the offset but not usually the zone (e.g. +8 vs America/Los_Angeles) and so the DST rules you might expect to apply don't.

It's just generally a bad idea to conflate what offset was used in specifying the time in this string with what zone should be used in the logic applied to the resulting time. It lets the tail wag the dog. It also assumes a level of intentionality about the way the string is specified that most, say, API callers would find unexpected. Obviously there are cases where you do want that (which is why we have setZone in the first place), but in my experience, most of the time people do it, they're doing so in error and causing themselves unnecessary headaches, when what they really wanted to do was be very explicit about the zone. So I don't want to do something that makes these mistakes more ergonomic.

I don't understand your use case, and maybe you've thought everything through and you really do want setZone: true on every parse of every ISO string. I believe you. But I don't think it makes sense to add the feature for everyone else, the vast majority of whom will use it to mess up their programs. I'm convincible on this (i.e. that there is a compelling and widespread use case for this kind of thing), but, well, I need to be convinced.

But fortunately, I think just wrapping the function is very easy...