Ghost
Ghost copied to clipboard
Replace moment with luxon in core/frontend
Ghost makes heavy use of the moment and moment-timezone libraries all over our code bases. However, it's an extremely heavy dependency, we have difficulties getting the timezone to "stick" and the project itself essentially recommends not using it anymore: https://momentjs.com/docs/#/-project-status/
Instead of using Moment, we want to change to using Luxon. We're using Luxon because of its timezone capabilities - something the core of Ghost makes heavy use of. In smaller packages where we only need to manipulate dates, rather than creating, storing and retrieving them, we'll use date-fns.
For the main Ghost codebase, @daniellockyer has already laid some groundwork, added Luxon as a dependency and ensured the default timezone is set to UTC.
From here, we need to start replacing usages of moment with luxon.
This is a new thing for us, we haven't used the library in many places yet and so we don't know a lot about it. For that reason, we want to start on areas of the codebase where we can do the least harm - the frontend. If we end up displaying dates incorrectly for some reason on the frontend that's much easier to fix than if we store the wrong date!
How to work on this issue
The slow and steady approach to refactoring is the only thing that really works. Branches should be very short lived (<1 day if possible). Therefore, please do not attempt to do this entire refactor in a single PR. It is not possible for us to review!
Instead, please find a single file inside of core/frontend
, make the necessary changes, test and lint your work, then submit a PR. If you like, repeat that with another file.
Luxon provides a useful guide to help translate between code in moment
and luxon
, along with important functional differences.
Please follow our [contribution guidelines] closely, particularly being sure to reference this issue in your commit. Refactor commits should not use emojis as they are not user-facing changes.
Thank you 🙏 ❤️ 🙏
@ErisDS @matthanley any feedback on my PRs?
Our bot has automatically marked this issue as stale because there has not been any activity here in some time.
The issue will be closed soon if there are no further updates, however we ask that you do not post comments to keep the issue open if you are not actively working on a PR.
We keep the issue list minimal so we can keep focus on the most pressing issues. Closed issues can always be reopened if a new contributor is found. Thank you for understanding 🙂
Hey @danimajo I still really want to get this done, but it turned out to require a lot more knowledge & context than I hoped. I don't think our tests are going to do a good job of picking up if formats change slightly. I also think we need more context of exactly which luxon functions to use where and why.
I know a couple of your PRs have now been closed by the stale bot, that's my bad. I need to look at them and provide feedback. The issue is that I don't have enough experience with luxon myself yet to know if the changes are correct :(
If there's anyone out there with a bunch of experience with it who'd be willing to help making changes, triaging PRs or just setting up some good example usages, I'd love to hear from you :)
Our bot has automatically marked this issue as stale because there has not been any activity here in some time.
The issue will be closed soon if there are no further updates, however we ask that you do not post comments to keep the issue open if you are not actively working on a PR.
We keep the issue list minimal so we can keep focus on the most pressing issues. Closed issues can always be reopened if a new contributor is found. Thank you for understanding 🙂
I'm going to close this until we have a bit more context about how to do it & can support the effort.
If there's anyone intimately familiar with moment -> luxon migrations that's interested to help, I'm all :ear: