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.

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

[Typescript] Moment types have been updated #982

Closed
jovpet opened this issue Apr 19, 2022 · 3 comments
Closed

[Typescript] Moment types have been updated #982

jovpet opened this issue Apr 19, 2022 · 3 comments

Comments

@jovpet
Copy link

jovpet commented Apr 19, 2022

Moment-timezone version which you use:

Version:

0.5.34 (latest)

Issue description:

Original Moment types have been updated, in version 2.29.3, and due to that fact typescript complains:

TS2352: Conversion of type 'import("node_modules/moment-timezone/node_modules/moment/ts3.1-typings/moment").Moment' to type 'moment.Moment' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.   The types returned by 'toArray()' are incompatible between these types.     Type 'number[]' is not comparable to type '[number, number, number, number, number, number, number]'.       Target requires 7 element(s) but source may have fewer.

temporary fix would be to do conversion to unknown first, but this should be fixed

@gilmoreorless
Copy link
Member

As far as I can tell, these type errors are not caused by Moment Timezone itself. It's just that Moment Timezone is exposing an underlying problem: multiple versions of base Moment in the same project.

I also got type errors like this in a project using Moment (but not Timezone) when we upgraded to 2.29.3. It was caused by a dependency requiring a different version of Moment from the main project, and they were conflicting.

In this case, when you load the default moment-timezone (via import or require), Moment Timezone will run require('moment') as the first action. Which version of moment gets loaded here depends a lot on the project's setup and package manager.

Why does this happen?

It comes down to package version resolution when your project depends on a package, and other packages also depend on that package. Ideally they all use the exact same copy of that dependency, but sometimes multiple versions are needed to satisfy all the version range requirements. Or sometimes a single version could be used, but dependency upgrades over time have caused the dependencies to get out of sync.

A simple example of including both moment and moment-timezone would look like this:

project
├─ moment (version A)
└─ moment-timezone
   └─ moment (version B)

You want moment versions A & B to be the same, but that doesn't always happen.

Most of the time, the two different versions don't cause much of a problem because they're fairly compatible. (Though if you're bundling for a front-end project, you now have 2 copies of moment in your bundle, leading to a lot of unnecessary code being loaded.)
But when there's a change to the type definitions like in moment/moment#5766, those two versions now have slightly different type signatures.

Getting an object from moment-timezone's copy of moment (version B) and passing it to a function that's expecting an object from the top-level moment (version A) will throw this type error.

(If your package.json only lists moment-timezone but NOT moment, then there's likely another dependency somewhere that also relies on moment. If the two packages are depending on different versions of moment, the type definitions will also clash.)

The best solution is to try to de-duplicate your copies of moment and get everything loading a single version. How to do that depends on which package manager you're using.

How to fix it

My experience is only with npm and yarn, so I can't give useful advice on anything else like pnpm.

npm

Expand for full details

First, find out what versions of moment are being included in your project using npm ls moment:

# Example
$ npm ls moment
mtz-ts-example@1.0.0 /path/to/project
├─┬ moment-timezone@0.5.34
│ └── moment@2.29.3
└── moment@2.29.1

If there's more than one version listed (like 2.29.1 and 2.29.3 above), that's where the conflict is. You can use the npm dedupe command to try to combine the dependency versions if possible.

NOTE: npm dedupe doesn't let you specify a single package, so it will run on ALL your packages. Use with caution.

$ npm dedupe
changed 1 package, and audited 4 packages

$ npm ls moment
mtz-ts-example@1.0.0 /path/to/project
├─┬ moment-timezone@0.5.34
│ └── moment@2.29.3 deduped
└── moment@2.29.3

Yarn

Expand for full details

Like the npm example above, you can use npm ls moment to find the used versions. Or there's also yarn why moment:

# Using Yarn v1
$ yarn why moment
yarn why v1.22.17
[1/4] 🤔  Why do we have the module "moment"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "moment@2.29.1"
info Has been hoisted to "moment"
info This module exists because it's specified in "dependencies".
info Disk size without dependencies: "5.14MB"
info Disk size with unique dependencies: "5.14MB"
info Disk size with transitive dependencies: "5.14MB"
info Number of shared dependencies: 0
=> Found "moment-timezone#moment@2.29.3"
info This module exists because "moment-timezone" depends on it.
info Disk size without dependencies: "5.16MB"
info Disk size with unique dependencies: "5.16MB"
info Disk size with transitive dependencies: "5.16MB"
info Number of shared dependencies: 0

The way to de-duplicate the versions depends on which version of Yarn you're using.

  • For Yarn v1 (Classic), there's a yarn-deduplicate tool that can be run via npx (which is bundled with Node.js):
    $ npx yarn-deduplicate --packages moment
    $ yarn install
    $ yarn why moment
    yarn why v1.22.17
    [1/4] 🤔  Why do we have the module "moment"...?
    [2/4] 🚚  Initialising dependency graph...
    [3/4] 🔍  Finding dependency...
    [4/4] 🚡  Calculating file sizes...
    => Found "moment@2.29.3"
    info Has been hoisted to "moment"
    info Reasons this module exists
    - Specified in "dependencies"
    - Hoisted from "moment-timezone#moment"
    info Disk size without dependencies: "5.14MB"
    info Disk size with unique dependencies: "5.14MB"
    info Disk size with transitive dependencies: "5.14MB"
    info Number of shared dependencies: 0
  • For Yarn v2 (Berry), the dedupe command is included by default.
    $ yarn dedupe moment
    ➤ YN0000: ┌ Deduplication step
    ➤ YN0000: │ No packages can be deduped using the highest strategy
    ➤ YN0000: └ Completed
    ➤ YN0000: ┌ Resolution step
    ➤ YN0000: └ Completed in 0s 365ms
    ➤ YN0000: ┌ Fetch step
    ➤ YN0019: │ moment-npm-2.29.1-787d9fdafd-1e14d5f422.zip appears to be unused - removing
    ➤ YN0000: └ Completed in 0s 313ms
    ➤ YN0000: ┌ Link step
    ➤ YN0000: └ Completed
    
    $ yarn why moment
    ├─ moment-timezone@npm:0.5.34
    │  └─ moment@npm:2.29.3 (via npm:>= 2.9.0)
    │
    └─ mtz-ts-example@workspace:.
    └─ moment@npm:2.29.3 (via npm:^2.29.1)

Review your importing strategy

The cases above are a very simplified version of a potentially very complex scenario. It's not always possible to neatly resolve version conflicts using tools and scripts.

A few of the bug reports about TypeScript errors in Moment Timezone have included code samples like this:

import 'moment-timezone';
import moment from 'moment';
import * as moment from 'moment';
import 'moment-timezone';

Importing both moment and moment-timezone in the same file is much more likely to cause version conflicts, for the reasons explained above. It's also redundant in most cases, because:

  1. moment-timezone imports/requires moment to extend its properties.
  2. moment-timezone's default export returns that extended moment object.

This is why the Moment Timezone docs examples only import/require moment-timezone and not core moment.
You will likely avoid a lot of potential TypeScript conflicts if you combine those imports into one:

import moment from 'moment-timezone';

This means you could also remove the direct dependency on moment from package.json (knowing that it will still be included by moment-timezone). But that depends on other uses and factors.

@jaybo
Copy link

jaybo commented Jul 15, 2022

@gilmoreorless Your comment was super helpful. Thanks!

@ichernev
Copy link
Contributor

Thank you @gilmoreorless, very professionally written and concise :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants