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

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

Closed
IllusionMH opened this issue Mar 5, 2019 · 7 comments · Fixed by #27
Closed

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

IllusionMH opened this issue Mar 5, 2019 · 7 comments · Fixed by #27

Comments

@IllusionMH
Copy link

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
Copy link
Owner

jeffijoe commented Mar 5, 2019

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

I am using the semver module for these checks.

packageInfo.versions.find(v => satisfies(v.version, version)) ||

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

@IllusionMH
Copy link
Author

Thanks!
Will try to prepare PR this weekend.

@devinrhode2
Copy link

@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
Copy link
Author

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.

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

Successfully merging a pull request may close this issue.

3 participants