Bump moment-timezone

[sainthkh]

Closes #10870.

It happened because there were 2 versions of moment-timezone installed in the project.

• There's a clear use-case for this code change
• Commit message has a short title & references relevant issues
• The build will pass (run yarn test and yarn lint)

More info can be found by clicking the "guidelines for contributing" link above.

[kevinansfield]

@sainthkh moment-timezone is intentionally held back due to buggy behaviour when upgrading that is not picked up by tests. There needs to be a deeper investigation and heavily verified behaviour because those bugs cause invalid date/time data to be stored in the database with a lot of knock-on effects and difficult recovery.

It happened because there were 2 versions of moment-timezone installed in the project.

Only 1 version is currently installed by forcing the older version in package.json resolututions. This PR changes that to introduce 2 versions, see https://github.com/TryGhost/Ghost/pull/12201/files#diff-8ee2343978836a779dc9f8d6b794c3b2L6209-R6210. Dependency updates need to be followed throughout all sub-dependencies to avoid multiple versions being referenced.

[sainthkh]

@kevinansfield

Thanks for telling me about resolutions. I was wondering why yarn creates an item about 0.5.23 which no dependency depends on. It's solved by updating that to 0.5.31.

When moment-timezone is updated to 0.5.31, 0.5.23 is cached under @tryghost/kg-default-cards package because it uses @tryghost/url-utils@^0.6.14 which depends on moment-timezone 0.5.28.

After reading the yarn.lock file, I realized that that is the only case where moment-timezone is used. So, I decided to clean up the versions of @tryghost/url-utils and moment-timezone. But 0.5.23 wasn't removed clearly because of the resolutions and it's now fixed.

Now, we have only one moment-timezone.

Ghost/yarn.lock

Lines 6197 to 6202 in 308e2c5

	moment-timezone@0.5.31:
	version "0.5.31"
	resolved "https://registry.yarnpkg.com/moment-timezone/-/moment-timezone-0.5.31.tgz#9c40d8c5026f0c7ab46eda3d63e49c155148de05"
	integrity sha512-+GgHNg8xRhMXfEbv81iDtrVeTcWt0kWmTEY1XQK14dICTXnWJnT0dxdlPspwqF3keKMVPXwayEsk1DI0AA/jdA==
	dependencies:
	moment ">= 2.9.0"

[kevinansfield]

@sainthkh having two versions is not the source of the problem. There are changes in moment-timezone and moment in the higher versions that cause bugs due to global timezone setting not being respected

[sainthkh]

Actually, it is.

Let me explain in more detail.

moment-timezone ignoring the global setting below has nothing to do with its updates.

Ghost/core/server/overrides.js

Lines 18 to 25 in 8aac74b

	/**
	* force UTC
	* - you can require moment or moment-timezone, both is configured to UTC
	* - you are allowed to use new Date() to instantiate datetime values for models, because they are transformed into UTC in the model layer
	* - be careful when not working with models, every value from the native JS Date is local TZ
	* - be careful when you work with date operations, therefor always wrap a date into moment
	*/
	moment.tz.setDefault('UTC');

To prove this, let's experiment this:

1. Create 2 Ghost clones with git clone.
2. In one repo, bump the version of moment-timezone to 0.5.31 (let's call this repo Ghost1). And leave the other repo as-is (let's call this repo Ghost2).
3. yarn to install dependencies in both repos.
4. Open node_modules folder of Ghost1 and find moment-timezone.
5. Copy the content of moment-timezone under node_modules in Ghost2 to Ghost1's moment-timezone.
6. Now, the code of moment-timezone inside Ghost1 uses the version 0.5.23.
7. Run the test by grunt test:regression\importer\importer_spec.js.
8. It fails. Even when we used the code of 0.5.23.
9. When you do the reverse, using 0.5.31 code inside Ghost2, it passes.

This 2 experiments tell us that the bug has nothing to do with the version of the moment-timezone.

---

Now, let's check what happens when we change the version number from 0.5.23 to 0.5.31 in the package.json.

Then, there's an another copy of moment-timezone under @tryghost/kg-default-cards folder. It's because it wants to use the version 0.5.28 of moment-timezone while the main Ghost project tries to use 0.5.31.

This happens a lot in nodejs projects and it is not a problem in most cases. But moment-timezone and moment are a bit different.

---

In momentjs constructor code below, there is a function called updateOffset.

// Moment prototype object
export function Moment(config) {
    copyConfig(this, config);
    this._d = new Date(config._d != null ? config._d.getTime() : NaN);
    if (!this.isValid()) {
        this._d = new Date(NaN);
    }
    // Prevent infinite loop in case updateOffset creates new moment
    // objects.
    if (updateInProgress === false) {
        updateInProgress = true;
        hooks.updateOffset(this);
        updateInProgress = false;
    }
}

This is defined as empty in momentjs like below:

// This function will be called whenever a moment is mutated.
// It is intended to keep the offset in sync with the timezone.
hooks.updateOffset = function () {};

But it is defined in moment-timezone to calculate the timezone:

	moment.updateOffset = function (mom, keepTime) {
		var zone = moment.defaultZone,
			offset;

		if (mom._z === undefined) {
			if (zone && needsOffset(mom) && !mom._isUTC) {
				mom._d = moment.utc(mom._a)._d;
				mom.utc().add(zone.parse(mom), 'minutes');
			}
			mom._z = zone;
		}
		if (mom._z) {
			offset = mom._z.utcOffset(mom);
			if (Math.abs(offset) < 16) {
				offset = offset / 60;
			}
			if (mom.utcOffset !== undefined) {
				var z = mom._z;
				mom.utcOffset(-offset, keepTime);
				mom._z = z;
			} else {
				mom.zone(offset, keepTime);
			}
		}
	};

This function overrides the momentjs's updateOffset.

Note: offset means "time zone" in momentjs. utcOffset function is used to define getSetZone function.

The problem with these 2 copies of moment-timezone happens with this overriding. Because we need to call the root moment-timezone's updateOffset to make global setting work correctly. But actually called updateOffset is from the one under @tryghost/kg-default-cards.

Because of this, the global setting was ignored. And I fixed this by reorganizing yarn.lock.

---

As you might have noticed, this PR passed all the tests. It means this PR bumps moment-timezone to the latest version and respects the global timezone setting of Ghost.

This PR meets all 4 conditions of the issue:

• bump moment-timezone package version to the latest in core => done.
• bump moment-timezone package version in SDK => It was already done before I started this PR.
• make sure default UTC timezone works globally => The test passes.
• remove moment-timezone from Renovate ignore list => I removed it.

---

After writing all this, I realized that my summaries in the original comment of this PR and the commit message were too brief to explain why it solves the problem.

I hope this explains why this is the solution to the problem.

And thank you for the review. If there's anything I missed, please tell me. Let's fix it together.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.