Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update moment-timezone dependency #10870

Closed
4 tasks
naz opened this issue Jul 4, 2019 · 5 comments
Closed
4 tasks

Update moment-timezone dependency #10870

naz opened this issue Jul 4, 2019 · 5 comments
Labels
dependencies Pull requests that update a dependency file server / core Issues relating to the server or core of Ghost

Comments

@naz
Copy link
Member

naz commented Jul 4, 2019 •
edited

Context

After Renovate bumped moment-timezone to 0.5.25 from 0.5.23 one of the regression tests started failing. The reason test failed, was a change in format() output for this assertion. The format was using local timezone instead of globally set 'UTC' one. The version in which the behavior change was introduced in moment-timezone 0.5.24. Was unable to pin down/replicate the issue in a clean environment. Added description of initial findings mentioned in the issue of moment-timezone repository.

TODO

The version bump will be reverted for now and added to ignore list in Renovate until the original issue is better understood. For someone who picks this issue up should make sure:

  • bump moment-timezone package version to the latest in core
  • bump moment-timezone package version in SDK
    (Revert TryGhost/SDK@5f0fc04)
  • make sure default UTC timezone works globally
  • remove moment-timezone from Renovate ignore list

NOTE: When updating to a newer version please pay attention to performance changes as moment is known to be one of the main perf. bottlenecks in Ghost.
@naz naz added help wanted [triage] Ideal issues for contributors to help with dependencies Pull requests that update a dependency file labels Jul 4, 2019
naz added a commit that referenced this issue Jul 4, 2019
@naz
Reverted moment-timezone bump back to 0.5.23
5aa0a21 
refs #10870

- Added moment-timezone to Renovate's ignore list
- Described reasoning  in #10870
@naz naz added the server / core Issues relating to the server or core of Ghost label Jul 4, 2019
rishabhgrg added a commit that referenced this issue Jul 30, 2019
@rishabhgrg
Added resolution for moment-timezone version
ffbd749 
refs #10870

- `moment-timezone` was bumped to `0.5.26` inadvertently as a result of bump to `url-utils` in 6cb0f80
- Added resolution makes sure we use `0.5.23` for `moment-timezone` till tests are updated to work with latest version
rishabhgrg added a commit to TryGhost/SDK that referenced this issue Jul 30, 2019
@rishabhgrg
Reverted moment-timezone to older version
5f0fc04 
refs TryGhost/Ghost#10870

- Reverts moment timezone to `0.5.23`
rishabhgrg added a commit that referenced this issue Jul 30, 2019
@rishabhgrg
Updated dependency @tryghost/url-utils to v0.1.4
86c670a 
refs #10870

- Reverts moment-timezone version in url-utils to 0.5.23 to fix moment format issue
@daviddarnes daviddarnes mentioned this issue Oct 2, 2019
Closed
@chanceeakin
Copy link

Pulled down and attempted a version bump to moment-timezone@0.5.26. Issue still persists in core during regression tests. Leaving this breadcrumb to save the next person a few minutes.

 1) Integration: Importer
       Empty database (except of owner user), general tests
         cares about invalid dates and date formats:

      AssertionError: expected '2013-10-18T18:58:44-05:00' to equal '2013-10-18T23:58:44Z'
      + expected - actual

      -2013-10-18T18:58:44-05:00
      +2013-10-18T23:58:44Z

    at Assertion.fail (node_modules/should/cjs/should.js:275:17)
    at Assertion.value (node_modules/should/cjs/should.js:356:19)
    at core/test/regression/importer/importer_spec.js:180:83
    at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:729:18)
    at _drainQueueStep (node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (node_modules/bluebird/js/release/async.js:102:5)
    at Immediate.Async.drainQueues (node_modules/bluebird/js/release/async.js:15:14)

@AlphaWong AlphaWong mentioned this issue Oct 16, 2019
Closed
AlphaWong added a commit to AlphaWong/Ghost that referenced this issue Oct 16, 2019
@AlphaWong
UPT: update and add default UTC timezone TryGhost#10870
6640fc6
@AlphaWong
Copy link

AlphaWong commented Oct 16, 2019 •
edited

I open a PR for it
#11245

plz take a look if it can resolve the issue for not ?

kevinansfield added a commit that referenced this issue Oct 29, 2019
@kevinansfield
Revert "Update dependency @tryghost/url-utils to v0.6.11"
37e4fd9 
This reverts commit 37e8951.

- the sub-dependency bump of `moment-timezone` caused another occurrence of #10870

# Conflicts:
#	package.json
@stale
Copy link

stale bot commented Jan 22, 2020

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.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Jan 22, 2020
@naz naz removed the stale [triage] Issues that were closed to to lack of traction label Jan 23, 2020
@stale
Copy link

stale bot commented Apr 23, 2020

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.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Apr 23, 2020
@naz naz added pinned [triage] Ignored by stalebot and removed stale [triage] Issues that were closed to to lack of traction labels Apr 27, 2020
@ErisDS ErisDS removed the help wanted [triage] Ideal issues for contributors to help with label Aug 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Sep 11, 2020
@sainthkh
Bump moment-timezone.
fa83c8d 
issue TryGhost#10870

The cause of the problem: there were 2 installations of moment-timezone. 
And moment-timezone overrides updateOffset function of moment. And the moment was calling the wrong one because there were 2 of them. 

I fixed yarn.lock to install only one of them.
sainthkh added a commit to sainthkh/Ghost that referenced this issue Sep 11, 2020
@sainthkh
Remove moment-timezone from renovate.json.
d8bed3a 
issue TryGhost#10870
@sainthkh sainthkh mentioned this issue Sep 11, 2020
Closed
3 tasks
naz pushed a commit to naz/Ghost-SDK that referenced this issue Sep 24, 2020
@rishabhgrg @naz
Reverted moment-timezone to older version
74d33c0 
refs TryGhost/Ghost#10870

- Reverts moment timezone to `0.5.23`
naz pushed a commit to naz/Ghost-SDK that referenced this issue Sep 28, 2020
@rishabhgrg @naz
Reverted moment-timezone to older version
8b8f0fe 
refs TryGhost/Ghost#10870

- Reverts moment timezone to `0.5.23`
naz pushed a commit to naz/Ghost-SDK that referenced this issue Sep 28, 2020
@rishabhgrg @naz
Reverted moment-timezone to older version
1c86583 
refs TryGhost/Ghost#10870

- Reverts moment timezone to `0.5.23`
@ErisDS ErisDS removed the pinned [triage] Ignored by stalebot label Nov 23, 2021
@ErisDS
Copy link
Member

ErisDS commented Nov 23, 2021

We're going to switch to luxon instead, see #13648

@ErisDS ErisDS closed this as completed Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file server / core Issues relating to the server or core of Ghost
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bump moment-timezone sainthkh/Ghost
4 participants
@ErisDS @naz @AlphaWong @chanceeakin