Decouple moment-timezone from tzdata

[BenAtAmplero]

The coupling of moment-timezone with tzdata significantly increases the test burden when we simply want to update tzdata.

I'm open taking this work on myself if an approach can be agreed upon.

[butterflyhug]

There are already instructions for downloading data from a new tzdb version and packaging that data with any version of the code. So you could just download the version of the code you've been using previously and use the scripts to re-package that code with new data.

It's also possible to use the no-data build of the library and then call moment.tz.load(data) with the JSON-parsed contents of a data file. The data format changes quite slowly, so you should generally be able to use data files from newer releases with older releases of the code.

Is there anything else we could provide to better serve your use case here?

[butterflyhug]

Suggestion from @schmod:

I think there's a reasonable case to be made for decoupling moment-timezone from the accompanying IANA timezone data (as a separate repo/NPM module)

[BenAtAmplero]

Both of those suggestions effectively fork moment-timezone because we need to manage either moment-timezone+updated tzdata or package the TZ data and some code to force the loading of that tzdata.

That said, you're suggestions are workable for now and much appreciated. Thanks!

[schmod]

I don't think a fork is necessary. moment-timezone can depend upon a separate tzdata package, and NPM can take care of the rest.

The only question would be how to version the bundled builds.

Right now, I think that we also need to consider that the implicit bundling of the library+data (which is very succinctly summarized in index.js) is posing problems for browser-based ES6/CJS users (ie. #364), because the default dataset is way too heavy for browser-based users.

[mattjohnsonpint]

I'd be fine with distributing moment-timezone as one package, with a dependency on another moment-timezone-data package. That would at least remove the need to increment the moment-timezone version with each tzdb release.

We may need some control over acceptable versions of the dependency, such as if we were to ever change the data format, but I think is is easily doable with npm.

WRT heaviness - It's no big deal for a node.js user to take on the entire data set, which is why the current npm package includes it. Browser users do indeed have different requirements though. Ideally, only the minimal data necessary for the application would be included, but clearly folks tend to just use the whole thing.

The moment-timezone docs haven't kept up with the newfangled ways the folks use package managers to download code, and frankly - neither have I. Please help me understand how changing what's in the npm package would affect browser-based users, and how we might be able to deliver different code to each environment with the same sets of packages. Are there other approaches we should consider?

Thanks.

[BenAtAmplero]

I'm a strong proponent of @mj1856's solution. moment-timezone depends on a newly created moment-timezone-data.

[schmod]

The big problem is that index.js includes the entire dataset as a hard-coded dependency. Like Node, any module-bundler is going to resolve the dependency and include the entire thing.

There are a few options we can take:

• Decouple the library from the data completely. By default, the library would ship with no tzdata. moment-timezone-data is a separate package, but is neither a dependency nor a peerDependency of moment-timezone. Users will be expected to manually load data and call tz.load().
  • Benefits: Everybody uses the same syntax. Data packages are completely independent, and versioned separately.
  • Drawbacks: Everybody uses the same rather verbose syntax. Unclear if there's a reasonable way for NPM to guarantee compatibility between versions of the library and the data package.
• Add "sub-modules" that include no/minimal data for browser users. Browser users can write require('moment-timezone/minimal') or require('moment-timezone/no-data').
  • Benefits: This is a straightforward CJS equivalent of the existing precompiled bundles. Users who want a smaller build can get one with minimal fuss. There's nothing browser-specific about this approach, so isomorphic applications can still run the same code in a server environment.
  • Drawbacks: If I accidentally write require('moment-timezone') anywhere in my code, I'll get the full bundle. This seems like an easy mistake to make, and I can't control what data will be loaded by any 3rd-party libraries that depend on moment-timezone.
• "Loader" API for browser users. At runtime, users can configure how moment-timezone can retrieve data for unknown timezones and date-ranges (presumably via an AJAX call).
  • Benefits: moment-timezone can lazily load tzdata as it's needed, and can fetch the "big" package if we need data about a date range outside of 2010-2020.
  • Drawbacks: Until System.import() is supported everywhere, there's likely no good way to implement this without creating adapters for each module format (ie. something separate for webpack, browserify, systemjs, require, etc.). Users would need to configure the global moment-timezone object to use the loader at runtime.

---

In my opinion, any solution should satisfy the following criteria:

• The application logic of moment-timezone should maintain a single API, regardless of whether the library is running in Node or a browser.
• There should be a reasonably-straightforward and well-documented path for developers using a module-bundler to include moment-timezone with no data, or minimal data.

---

Now, the elephants in the room:

moment-timezone mutates the global moment object. This is bad for TypeScript users, and also makes it difficult to write well-encapsulated code. It shouldn't be possible to access moment.tz via require('moment').

Worse still, moment.tz is stateful and mutable. If I require('moment-timezone'), I don't have any guarantees about what locales other scripts have previously defined via moment.tz.load() or moment.tz.add().

There may not be any good solutions to these issues -- I suspect that many developers would find it unreasonable to need to call moment.tz.load() from every module. But, it's worth noting that this contributes to our problems.

[maggiepint]

We should finish the work cleaning up the moment-timezone interface and how it interacts with moment core. That would clean up a lot of the concerns about the mutation of the core.

[schmod]

A possible compromise:

The default export of moment-timezone becomes a factory that returns a moment object decorated with .tz, and the exact set of locales that we've defined. The object returned by require('moment') is never mutated.

It could be used as follows:

const tzdata = require('moment-timezone-data');
const moment = require('moment-timezone')(tzdata);

From there, we could extend this to resemble a builder pattern, moving the add / link methods off moment.tz constructor, and onto our factory:

const moment = require('moment-timezone')
    .add('America/Los_Angeles|PST PDT|80 70|0101|1Lzm0 1zb0 Op0')
    .add('America/New_York|EST EDT|50 40|0101|1Lz50 1zb0 Op0')
    ();

We could also place an extension point here to allow users to define an asynchronous loader:

const moment = require('moment-timezone')
   .add('America/Los_Angeles|PST PDT|80 70|0101|1Lzm0 1zb0 Op0') // I know I need this one.
   .fallback(function(year, zone, cb){
       var decade = +year.toPrecision(3);
       var url = "http://example.com/locales/tz/" + decade + "/" + zone + ".json";
       fetch(url).then( response => cb(response.json) );
   })();

(This would admittedly require all of the moment.tz methods to be asynchronous, which probably isn't worth the effort.)

---

With this pattern, there should be no surprises at runtime, because instances of the moment and moment.tz constructors are effectively immutable.

However, there are some drawbacks:

• The code is uglier and more verbose.
• It's unclear if this is actually a problem. Timezone data doesn't change all that frequently.
• Libraries depending on moment-timezone would need their consumers to supply tzdata.

[schmod]

@maggiepint -- That sounds good. I agree that this is the best place to start before we tackle the other issues at hand.

[quentin-sommer]

@schmod Can I find some updates on this somewhere?

[evanjmg]

+1