Custom Timezones only work in certain circumstances

[gordoncl]

Describe the bug

I'm not entirely sure if this is a bug or if this isn't intended to be a supported feature, but it seems like custom timezones are poorly supported (or at the very least, poorly documented). Particularly, there are a few things I'd like to point out about the behavior:

1. There are discrepancies between when the isUniversal flag is true or false.
2. There are discrepancies between when a custom timezone is overriding an existing system timezone.

I did get the impression that custom timezones should be supported from reading this blurb about specifying a zone.

A custom implementation of Luxon's Zone interface (advanced only)

At the very least, I do think that if something comes out of this issue, it should be at least to update the documentation and perhaps add an example of how to implement custom timezones and their limitations. Or, to completely remove the blurb all together.

To Reproduce

Below is a basic set up for a custom timezone. In this case the behavior for isUniversal is not actually honored, all times will be offset by one hour after 3600000.

There are four supported timezones, two of which are overrides: Bogus/NotUniversal, Bogus/Universal, America/New_York, America/Los_Angeles.

const { DateTime, FixedOffsetZone, Zone } = require('luxon');

const ONE_HOUR = 3600000;

class MyZone extends Zone {
  /**
   * Determines whether or not the time zone is universal.
   */
  static UNIVERSAL = {
    'Bogus/NotUniversal': false,
    // Overrides a known time zone.
    'America/New_York': false,
    'Bogus/Universal': true,
    // Overrides a known time zone.
    'America/Los_Angeles': true,
  }

  constructor(name) {
    super();
    this._name = name;
  }

  get isUniversal() {
    return MyZone.UNIVERSAL[this._name];
  }

  get isValid() {
    return true;
  }

  get name() {
    return this._name;
  }

  get type() {
    return 'custom';
  }

  equals(zone) {
    return zone.name === this._name;
  }

  offset(ms) {
    // After January 1, 1970 01:00 UTC, set the clocks forward an hour.
    return ms >= ONE_HOUR ? 60 : 0;
  }

  offsetName() {
    return 'N/A';
  }

  formatOffset(...args) {
    return FixedOffsetZone.prototype.formatOffset(...args);
  }
}

Actual vs Expected behavior

Again, I'm not sure what is a bug here and what isn't, it just appears there are just a lot of inconsistencies in behavior. So I'm just going to print what occurs:

let base = DateTime.fromMillis(0).setLocale('en-US');

// Jan 1, 1970, 12:00 AM
console.log(base.setZone(new MyZone('America/Los_Angeles')).toLocaleString(DateTime.DATETIME_MED));

// Dec 31, 1969, 7:00 PM
console.log(base.setZone(new MyZone('America/New_York')).toLocaleString(DateTime.DATETIME_MED));

// Jan 1, 1970, 12:00 AM
console.log(base.setZone(new MyZone('Bogus/Universal')).toLocaleString(DateTime.DATETIME_MED));

// Uncaught RangeError: Invalid time zone specified: Bogus/NotUniversal
console.log(base.setZone(new MyZone('Bogus/NotUniversal')).toLocaleString(DateTime.DATETIME_MED));

base = base.plus({ milliseconds: ONE_HOUR });

// Jan 1, 1970, 2:00 AM
console.log(base.setZone(new MyZone('America/Los_Angeles')).toLocaleString(DateTime.DATETIME_MED));

// Dec 31, 1969, 8:00 PM
console.log(base.setZone(new MyZone('America/New_York')).toLocaleString(DateTime.DATETIME_MED));

// Jan 1, 1970, 2:00 AM
console.log(base.setZone(new MyZone('Bogus/Universal')).toLocaleString(DateTime.DATETIME_MED));

// Uncaught RangeError: Invalid time zone specified: Bogus/NotUniversal
console.log(base.setZone(new MyZone('Bogus/NotUniversal')).toLocaleString(DateTime.DATETIME_MED));

Some notes:

• When isUniversal is true the behavior always seems to produce the results I would expect, regardless of whether we are overriding a system timezone or adding a custom one.
• When isUniversal is true and the timezone is overriding a system timezone, it seems like the system time is used.
• When isUniversal is false and the timezone is not overriding a system timezone, formatting the date causes an exception to be thrown.

Additional context

For my particular use case, I would like to be able to override existing timezones and potentially add new ones if our version of tzdata is higher than that of the systems.

[icambron]

There are two things happening here: the first is a bug in your class and the other is a bug in Luxon.

But first, let's clear up a misunderstanding: isUniversal is a flag that tells Luxon "the offset is always the same for this zone". That doesn't mean it's not going to call the code that computes the offset. It has to do that to find out the offset. It just assumes that once it knows the offset for a zone, it never has to ask again. This has a few applications scattered throughout Luxon. But it's not really something that can be "honored" or "not honored". It's more like an optimization.

OK, onto the actual issues: the first is the equality operator in your custom zone. return zone.name === this._name; will return true when compared to any zone with the same name, whether it's a MyZone or not. setZone() noops if the zone is already equal to the one passed in. This is hard to see in your example but your base var is already in NY time and Luxon ignores the passed-in zone because it think it's the same, based purely on the equality check. If we cut out the toLocaleString() stuff and just print the times, we get this:

let base = DateTime.fromMillis(0).setLocale('en-US');
console.log(base.setZone(new MyZone('America/Los_Angeles')).toISO()); //=> 1970-01-01T00:00:00.000Z
console.log(base.setZone(new MyZone('America/New_York')).toISO()); //=> 1969-12-31T19:00:00.000-05:00 

That second one is totally ignoring your custom zone, which doesn't know that NY is ever at -5.

You need to not just check the name but also ensure that the passed-in zone is a MyZone. So let's fix that and then remove the toLocaleString stuff and just see what dates we get:

let base = DateTime.fromMillis(0).setLocale('en-US');
console.log(base.setZone(new MyZone('America/Los_Angeles')).toISO()); //=> 1970-01-01T00:00:00.000Z
console.log(base.setZone(new MyZone('America/New_York')).toISO()); //=> 1970-01-01T00:00:00.000+00:00  

base = base.plus({ milliseconds: ONE_HOUR });
console.log(base.setZone(new MyZone('America/Los_Angeles')).toISO()); // => 1970-01-01T02:00:00.000+01:00
console.log(base.setZone(new MyZone('America/New_York')).toISO());  //=> 1970-01-01T02:00:00.000+01:00

That looks as expected. You aren't actually doing anything differently between NY and LA, so they are getting the same times. The isUniversal is causing Luxon to show a Z instead of a +00:00. The custom offset logic is working too.

But once we put back in the toLocaleString() calls, everything stops making sense again. The second issue is that Luxon has a pretty significant bug in how it handles custom zones in toLocaleString() and other formatters. Specifically: it thinks that if the zone is not a system zone and also not universal, then the formatter can feed the zone's name property directly into the Intl formatter as the timeZone argument. This is an invalid assumption and is causing two problems:

1. If the zone name isn't a valid IANA zone, Intl chokes on it. This is the problem for your Bogus/NotUniversal zone
2. If it is valid, the formatter uses it to spit out the date, and Intl does its own computation using its idea of what "America/New_York" (or whatever) means. This has the effect that for formatting, your custom zone is being ignored if it's not universal.

This is where it gets tricky. Luxon relies on Intl to spit out the string, but Intl doesn't understand custom zones. I suspect to fix this Luxon will need to treat custom zones as a fundamentally different thing. (Implementation notes for whoever fixes this: probably the best way to do this is to treat custom zones just like fixed-offset ones; i.e. we pass in the GMT zone nam to the intl formatter. This spares us doing a keepLocalTime calculation. It means, of course, that we can't accurately handle formatting the zone itself, but there's no way to do that anyway, since Intl simply doesn't understand our zone).

Finally, in terms of custom zones having better documentation and some more thorough guidance: completely agree. Someone just needs to do the work.