Skip to content
This repository was archived by the owner on Jun 27, 2020. It is now read-only.

Conversation

@deecewan
Copy link
Contributor

@deecewan deecewan commented Jul 25, 2018

Currently, when the timezone data is loaded, the loading function tries to enrich the metazone data which will allow for name aliases of the timezone to refer to the same metazone.

Unfortunately, the build script was concatenating the imports incorrectly so the TZ data was imported before the metazone data so the matching metazone was never found.

It's not possible (in my understanding of the code base) to write a test to assert this behaviour. In the tests, the metazone is run before the tzdata so it is in the correct order.

Before:

// build/src/index.js
/*
 * Copyright 2017, Yahoo Inc.
 * Copyrights licensed under the New BSD License.
 * See the accompanying LICENSE file for terms.
 */
var myGlobal = (typeof global !== "undefined" && {}.toString.call(global) === '[object global]') ? global : window;
(require('./code/polyfill.js').default)(myGlobal);
(require('./code/data-loader.js').default)(myGlobal);
(require('./data/tzdata.js'))(myGlobal);
(require('./data/metazone.js'))(myGlobal);
(require('./data/locale.js'))(myGlobal);
module.exports = myGlobal.Intl.DateTimeFormat;

After:

// build/src/index.js
/*
 * Copyright 2017, Yahoo Inc.
 * Copyrights licensed under the New BSD License.
 * See the accompanying LICENSE file for terms.
 */
var myGlobal = (typeof global !== "undefined" && {}.toString.call(global) === '[object global]') ? global : window;
(require('./code/polyfill.js').default)(myGlobal);
(require('./code/data-loader.js').default)(myGlobal);
(require('./data/metazone.js'))(myGlobal);
(require('./data/locale.js'))(myGlobal);
(require('./data/tzdata.js'))(myGlobal);
module.exports = myGlobal.Intl.DateTimeFormat;

@yahoocla
Copy link

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! πŸ˜„

@deecewan
Copy link
Contributor Author

fyi @juandopazo - not sure how this was never picked up before, but I was getting errors when running with the timezone name Australia/Canberra. After the changes, it works as expected.

@benmccann
Copy link
Contributor

@markandey would you be able to take a look at this PR?

@markandey markandey merged commit 4a9b6fb into formatjs:master Dec 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants