search

jeffijoe / typesync

Install missing TypeScript typings for dependencies in your package.json.
MIT License
1.51k stars 21 forks source link

typesync doesn't respect packages major and minor versions #25

Closed IllusionMH closed 5 years ago

IllusionMH commented 5 years ago

Firs of all - thank you for this easy to use tool. It found even more typing that I've added initially by hand.

However I've noticed a problem. While tool preserves "Semver ^ or ~ for a package" it looks like it doesn't check actual package version.

For example for package.json that has

"backbone": "1.1.2",
"bootstrap": "^3.3.7",

it added

"@types/backbone": "1.3.46",
"@types/bootstrap": "^4.3.0",

Which has correct range marker in version, but still installed incompatible types versions.

DT packages should match major and minor package versions of packages they are providing typings for, so it make sense to restrict @types package to compatible semver minor version (or lower like in case of @types/react-dom that doesn't have all minor version and had a lot of 16.0.x version until jumped to 16.8)

Also preserving semver major.minor version will allow to call npm and see if package isn't marked as deprecated (related to #24)

In case when there is no compatible versions, but higher versions available might be better ask (or provide CLI flag) to add them.

jeffijoe commented 5 years ago

I didn't think about this as I always keep my packages updated. 😇

This is going to make things a bit more complicated, but I'll look into it when I have some time to work on this.

jeffijoe commented 5 years ago

I'm currently working on this.

TypeSync is going to try to find the closest semver match, and if none are found, TypeSync will pick the newest (latest) typings version. This should not happen too often, so I think dealing with it on a case by case basis is fine.

IllusionMH commented 5 years ago

Checked with 0.5.1 and almost all cases are supported now.

Only problem that I've found is when exact version specified and came exact version of types exists. For "react": "16.7.0" typesync will add "@types/react": "16.7.0" to devDependencies, however there are version up to 16.7.22.

However, since DefinitelyTyped requires only major and minor version to match with package, and patch version is "free" for typing update - it makes sense for packages with exact range search for types in the same minor range.

jeffijoe commented 5 years ago

I am using the semver module for these checks.

https://github.com/jeffijoe/typesync/blob/f3da5037ea6142d91f4db47c22eaa0880f841d4d/src/type-syncer.ts#L168

If you disagree with the current behavior, feel free to submit a PR and I'll be happy to take a look. 😄

IllusionMH commented 5 years ago

Thanks! Will try to prepare PR this weekend.

devinrhode2 commented 2 years ago

@IllusionMH - did this PR ever happen and just not get linked, or did you have a change in perspective and decided not to make the change?

IllusionMH commented 2 years ago

I've changed perspective. Sorry about silence.

Main reason is that in projects we have lock files that are enforced so "dep": "^1.2.3" might have latest published version that matches ^ to be 1.9.0, but actual package version installed (as in lock file) an used by application can be 1.6.2.

typesync uses package.json files as only (IIRC) source of information about versions, but to be precise as in initial request implementation would need to start either checking package.json files of installed modules or support yarn.lock/package-lock.json (and track package version that was hoisted to the top).

Therefore after some attempts to get implementation for general case I've gave up as this change would be challenging to implement and then drastically expand scope and maintenance of package.

As we don't update packages too often to have constant need to use typesync - I've used typesync to get list of packages that have definitions which can be installed and then just manually checked actually used versions and installed corresponding version of @types.