Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Special handling for npm @types #519

Closed
ikatyang opened this issue Jul 24, 2017 · 28 comments
Closed

Special handling for npm @types #519

ikatyang opened this issue Jul 24, 2017 · 28 comments
Labels
help wanted Help is needed or welcomed on this issue manager:npm package.json files (npm/yarn/pnpm) priority-4-low Low priority, unlikely to be done unless it becomes important to more people type:feature Feature (new functionality)

Comments

@ikatyang
Copy link
Contributor

This is a common usage for plugin-like packages, the plugin package's version respect to target package version, for example:

Case 1

  • A: 1.2.3
  • A-plugin-X: 1.2.x, 1.x, etc.
    • plugin version may be lower than target package, but will never higher than target package (except patch version)

Case 2

I'm using [email protected] but there is @types/[email protected] available, which is not compatible with target package, I'd like to not upgrade @types/gulp until gulp is upgraded.

  • TypeScript's definition packages are always named @types/xxx (published by official BOT), and its version are same as target package's MAJOR.MINOR version.

Option may looks something like:

{
  "pluginPackages": {
    "A": {
      "LOCK.YES.YES": ["A-plugin-X"]
    },
    "(.+)": { // regex
      "LOCK.LOCK.YES": ["@types/$1"] // "lock-versions": ["template"]
    }
  }
}
@rarkins
Copy link
Collaborator

rarkins commented Jul 24, 2017

Isn't this the type of cross-package dependency that should be described using peerDependencies?

Because I'm thinking how to add a feature so that Renovate doesn't "break" peerDependencies and this sounds like the same type of thing.

@rarkins
Copy link
Collaborator

rarkins commented Jul 24, 2017

I checked @types/gulp and it does not list peerDependencies:

  "name": "@types/gulp",
  "peerDependencies": {},

This seems like two mistakes in the publishing:

  1. It depends on a minimum version of another package but that's not described in peerDependencies
  2. The other package isn't even published yet, so I think @types/gulp should be published as "next" tag and not "latest". Renovate would ignore anything that's not "latest".

That said, I am not against practical solutions to real problems, and if publishers do a bad job then that's indeed a "real problem" for users like yourself. But before doing such hacks I really want to make sure I understand everything 100% first.

@rarkins
Copy link
Collaborator

rarkins commented Jul 24, 2017

By the way, one of the most common plugin packages I've seen is eslint, and eslint plugins definitely don't follow the same major/minor version as eslint in many cases. And they also seem pretty good at specifying the right peerDependency eslint version. Can you point me to any examples of this or others so I can understand better?

@rarkins
Copy link
Collaborator

rarkins commented Jul 24, 2017

Here's the eslint devDependencies from this repo:

    "eslint": "3.19.0",
    "eslint-config-airbnb-base": "11.3.0",
    "eslint-config-prettier": "2.3.0",
    "eslint-plugin-import": "2.7.0",
    "eslint-plugin-prettier": "2.1.2",
    "eslint-plugin-promise": "3.5.0",

I think they all support eslint 4.x too - I need to check that before merging the eslint 4.x PR. So for eslint I definitely think peerDependencies is the way to handle it (but we don't handle it yet..)

@ikatyang
Copy link
Contributor Author

  1. It depends on a minimum version of another package but that's not described in peerDependencies

This is true, but for @types packages, it is impossible to use peerDependencies field since @types/A may have a dependency @types/B, and A uses B internally so that when you installed @types/A it'll always show that @types/B has a mismatching peerDependency. And @types packages can also be standalone for those web-package(web-only or UMD-like) definitions, for example: @types/googlemaps.

  1. The other package isn't even published yet, so I think @types/gulp should be published as "next" tag and not "latest". Renovate would ignore anything that's not "latest".

Agree, but they currently are not able to do such things, I may raise an issue there later.


@types is just a special case for plugin-like packages. For those general cases, I think peerDependency is the correct solution.

@rarkins
Copy link
Collaborator

rarkins commented Jul 25, 2017

It still seems a little hacky to me, but I'm willing to consider it if there's no other solution. So for @types/gulp you want the ability to say "never upgrade @types/gulp to a version greater than gulp version"? Or can it be simplified even more to say "match major" (i.e. don't upgrade to 4.x if gulp is on 3.x, and it's ok if it's > gulp version otherwise).

@rarkins rarkins added help wanted Help is needed or welcomed on this issue needs-requirements priority-4-low Low priority, unlikely to be done unless it becomes important to more people labels Jul 25, 2017
@ikatyang
Copy link
Contributor Author

Matching both major and minor would be better, but yeah, thanks for considering this suggestion.

Renovate is the best tool I've never seen before, I used to use Greenkeeper but it always makes a lot of PR and I have to merge them one by one manually, which is annoying, renovate's auto-merging save my life, I just wake up and "WOW, they're already updated ❤️ ", thanks again for the great tool. 👍

@rarkins
Copy link
Collaborator

rarkins commented Jul 25, 2017

OK, if I implement something like this then we should at least do it "right" and make it work for all cases. Something like a "followMajor", "followMinor" and "followPatch" idea could work. But I am still thinking how to implement this cleanly.

Q1: It sounds like you couldn't add a regex-based rule for all @types/* repositories because they don't all follow a "paired" dependency like gulp does? Or do you think you could define a general rule?

Q2: Do these need to be upgraded together? e.g. let's say that gulp 4.0.0 is finally released to npm and Renovate proposes it as a major upgrade. Would your build/project break if @types/gulp isn't upgraded together to 4.x at the same time?

Q3. I assume there must be people who are installing @types/gulp 4.x and also using gulp 4.x directly from GitHub? In that case I think any "automatic" rule for @types would be hard to enforce.

@ikatyang
Copy link
Contributor Author

A1: The @types definition package for npm packages are always "paired", so that I think the regex-based rule should be possible.

A2: It seems that's based on the package, if they follow the backward compatibility, upgrading them separately is fine, otherwise it might need to upgrade together. And it seems currently is not able to group them using some "mapping" matching. (e.g. group gulp and @types/gulp together)

A3: In that case, gulp is using GitHub-based dependency, something like "gulp": "gulpjs/gulp#4.0", which is not a npm dependency so Renovate can just ignore them I think, or can add some exclude option for the pluginPackages.

@rarkins rarkins added the manager:npm package.json files (npm/yarn/pnpm) label Oct 20, 2017
@rarkins
Copy link
Collaborator

rarkins commented Nov 10, 2017

@ikatyang I would like to revisit this issue, specifically for @types/* rather than as a generic solution. Now that you've been using renovate for quite a while, do you have any new ideas on how the logic could work? e.g. could it be:

  • If both @types/x and x exist in the same package.json then group them together into the same branch/PR
  • Never upgrade the @types/x major.minor higher than the same x major.minor?

As I am not a typescript user myself, I don't really understand it deeply. For TS users, do you usually wait for an update to @types/x before upgrading x? Or might you upgrade x as long as the "old" @types/x does not cause any test failures with the new x?

@rarkins
Copy link
Collaborator

rarkins commented Nov 10, 2017

BTW I'm thinking one quick hack can simply be for me to change the way branchName is constructed for @types/* packages. e.g. currently it's like renovate/types-node-4.x but I could add a special rule e.g. to use renovate/node-4.x instead, which would then implicitly group it. Also maybe you can tell me how you've been handling it with Renovate already without this feature yet?

@rarkins
Copy link
Collaborator

rarkins commented Nov 10, 2017

I looked here: https://github.com/ikatyang/prettylint/blob/master/package.json

Things I notice:

  1. It is made a little complicated because dependencies can be ranges (e.g. ^2.2.0) while the matching type definition is pinned (e.g. 2.2.0)
  2. "@types/jest" is behind by a minor release
  3. "@types/meow" is behind by a minor release
  4. "@types/node" has no pair (node >= 4 is defined in engines though)
  5. "@types/prettier" is ahead of prettier although it's a major range (^1.6.0)
  6. "@types/strip-ansi" is behind by a major release

Based on this, it seems really hard to have any "fixed" rules for upgrading or pairing them. Let me know if you can think of any!

@ikatyang
Copy link
Contributor Author

If both @types/x and x exist in the same package.json then group them together into the same branch/PR

👍

For TS users, do you usually wait for an update to @types/x before upgrading x? Or might you upgrade x as long as the "old" @types/x does not cause any test failures with the new x?

There shouldn't be problems for upgrading x before @types/x unless there's breaking changes, and also in TS we can add types to existed declarations dynamically so that it should be a blocker, and I usually do it this way before I send PRs to the @types/x repo.


Also maybe you can tell me how you've been handling it with Renovate already without this feature yet?

Never upgrade the @types/x major.minor higher than the same x major.minor?

It is made a little complicated because dependencies can be ranges (e.g. ^2.2.0) while the matching type definition is pinned (e.g. 2.2.0)

Based on this, it seems really hard to have any "fixed" rules for upgrading or pairing them. Let me know if you can think of any!

Rules I can think of:

  • lower version @types/x is fine.
  • never use the higher version @types/x unless it's already higher than x (major.minor, regardless of the range prefix).
  • @types/node should be paired with engine.node

Thanks for revisiting this issue ❤️

@rarkins
Copy link
Collaborator

rarkins commented Dec 7, 2017

This was recently suggested by @kamal too in #1276, He wrote:

Versioning schemes for @types packages typically follow the package they are typing. For example, graphql 0.10.x has @types/graphql 0.10.x and graphql 0.11.x has @types/graphql 0.11.x. This would make sense as a package group setting.

@rarkins
Copy link
Collaborator

rarkins commented Dec 7, 2017

Although both @ikatyang and @kamal have suggested "generic" approaches to solving this (non-types-specific), and although this is usually the way I like to approach it, I think I may attempt a types-specific feature first and try to get it right. Maybe later if we find another use case we can think of a way to make the configuration more generic.

One easy way I can group types together is to change the branchName for types so that instead of renovate/types-graphql-0.x it would be renovate/graphql-0.x. Doing so would mean it gets grouped with the graphql package whenever there is an upgrade for both available.

What it doesn't automatically do is limit the types package to being in "lock step" with the original, so I still need to think of an elegant way to do that. Would this logic work?

"If @types/<package> is being upgraded, and <package> also exists in the same package file, then skip the @types/<package> upgrade unless it has the same or lower minor version as the <package> has?"

There are still some challenging edge cases/race conditions to handle though, even if the above works. e.g. imagine you are currently on graphql:0.10.x and @types/graphql:0.9.x, and the graphql 0.11.x is released - but you are not upgrading yet. Later, @types/graphql releases 0.10.x and 0.11.x. Ideally you would take the 0.10.x but the logic for that is not so easy because we'd probably choose 0.11.x and ignore 0.10.x.

@rarkins
Copy link
Collaborator

rarkins commented Jan 10, 2018

@ikatyang @kamal I wanted to update you on a small improvement made today to how we handle @types/foo branches. Previously the branchName for would be something like renovate/types-foo-1.x. Now, it will instead be renovate/foo-1.x which will implicitly group it together in the same PR as foo v1.x upgrades, if both are present. Here is an example from another project:

image

The PR then looks like this:
image

I think - but need to confirm this with some hands-on testing - that it should also improve handling when a types package is available but exceeds your currently installed package:

  • if foo receives an update to v2, then a new PR will be created for it
  • If there is no corresponding @types/foo immediately, then it obviously won't be included in that initial PR
  • Once @types/foo reaches v2, it will be bundled together with the existing PR for foo
  • If that PR is closed then both foo and @types/foo upgrades to v2 will be skipped until you either update to it manually in future, or you rename the closed PR to trigger it being recreated

So I think the next step would be to suppress @types/foo major versions if they are in a PR where the major version is greater than foo's. e.g. let's say foo has a 4.0.0-alpha.3 available and so types@foo releases 4.0.0. Currently, you will get @types/foo even if you still have [email protected] for example. So in future, it would be nice to suppress that @types/foo v4 until you upgrade to foo v4 - either a alpha/beta or the final version.

@rarkins
Copy link
Collaborator

rarkins commented Jan 10, 2018

This does not yet solve the problem of minor version pairing, e.g. let's say you are on [email protected] with same version of the types. Then 0.11.0 is released but you are not ready to upgrade. Next, an @types/graphql releases 0.10.1 to fix a bug there.. unfortunately that would be skipped unless the user configures separatePatchReleases for types. Maybe that's an option?

@ikatyang
Copy link
Contributor Author

Great work! The logic looks good to me. 👍

Regarding minor versions, I think we can always upgrade @types/foo to the latest version that is not greater than foo regardless the difference between major/minor/patch, and foo will remain the current upgrade logic (just like what we did before bundled with @types).

P.S. the not greater than logic does not include patch versions for @types since there is no relationship between patch versions for @types/foo and foo, it's just a counter for @types PRs.

@felixfbecker
Copy link

Using peerDependencies has been suggested to types-publisher before: microsoft/types-publisher#13

I think a tool like Renovate wanting to programmatically match the supporting versions is a strong argument that it should be done.

Maybe someone from the TS team could chime in here, this would greatly improve the TS ecosystem if we can properly use tools like Renovate with TS projects. @RyanCavanaugh

@rarkins
Copy link
Collaborator

rarkins commented Jun 22, 2018

@felixfbecker thanks for bringing that to my attention. Having the @types packages declare the corresponding peer dependencies is slap-forhead-obvious in hindsight.

In the meantime, do you have any real examples of where Renovate proposed upgrading a @types package that was too far past its sibling package? I'm ready to take another pass at hacking together custom behaviour in Renovate while we wait for a "standardised" solution with peerDependencies.

@RyanCavanaugh
Copy link

@felixfbecker can you bring me up to speed on this?

@rarkins rarkins changed the title Ignoring version depend on another package version? Special handling for npm @types Jul 26, 2021
@rarkins
Copy link
Collaborator

rarkins commented Aug 8, 2021

I think we can finally achieve this using #11092 and the concept of groupPreference=parentRepo

@buffcode
Copy link

buffcode commented Sep 7, 2021

@rarkins Will grouped packages automatically pin to the same version? In my understanding Renovate would just group available updates into the same PR but there is not logical connection between the packages' version. The result would be the same as now (though more sophisticated) where @types/ is removed from the branch name in order to merge with the main package.

@simon-abbott
Copy link
Contributor

Has there been any movement on this recently? It would be really nice to have.

@bmaupin
Copy link

bmaupin commented Jan 17, 2023

I would love this! Here's my current situation:

Last week Renovate proposed this merge request:

  • @types/jsonwebtoken 8.5.9 -> 9.0.0
  • @types/node 14.18.29 -> 18.11.18
  • @types/react 17.0.50 -> 18.0.26
  • @types/react-dom 17.0.17 -> 18.0.10

I didn't want any of those because they don't match the major version of the packages I'm using. I also have 14 in .nvmrc. So I closed the merge request.

Today I have a new merge request:

  • @types/jsonwebtoken 8.5.9 -> 9.0.1
  • @types/node 14.18.29 -> 18.11.18
  • @types/react 17.0.50 -> 18.0.26
  • @types/react-dom 17.0.17 -> 18.0.10

As you can see only one package changed versions. Here are the issues as far as I can see them:

  • The behaviour between this merge request and a normal package merge request is different; if Renovate proposes a major upgrade to a package and I close the MR, it gives me this message:

    As this MR has been closed unmerged, Renovate will ignore this upgrade and you will not receive MRs for any future 18.x releases.

    But in this case, I received no such notification when I closed the first MR and Renovate is still creating merge requests for the same major version and the same package instead of ignoring them

  • As already discussed by the OP, Renovate is suggesting a different major version for the types package than the normal package

  • I have 14 set in .nvmrc and Renovate is not using that in its logic for the @types/node package

  • Ideally, the types packages themselves would be grouped not with all the other types packages but with the normal package they correspond to

I'll dig through the documentation and see if I can find a more explicit way of telling the Renovate bot to ignore these type packages until the normal packages are updated.

Thanks!

@rarkins
Copy link
Collaborator

rarkins commented Jan 17, 2023

Please raise a configuration discussion, it looks like you're maybe grouping the types packages major updates unnecessarily

@bmaupin
Copy link

bmaupin commented Jan 17, 2023

I see we're using

"extends": ["group:definitelyTyped"],

so I guess that explains the behaviour in the first bullet point. I'll remove that to get more normal behaviour.

And maybe this issue will handle the other three once it's resolved. Thanks!

@RichiCoder1
Copy link
Contributor

RichiCoder1 commented Mar 3, 2023

I don't have a good answer for this, butwould a possible implementation be a template based approach (with maybe a built in preset) ala regexManagers?

Absolutely terrible pseudo code and I'm not sure how it would workout in reality but something like:

{
	matchPackageNames: ["@types/my-package"],
    peerVersionTemplate: {
		peer: "my-package",
        // could be a peer template?
        // matchPeerString: "@types\/(?<peerName>.*)"
        // peerTemplate: "{{peerName}}"
		// maybe have peerMatchString ala regexManager.matchStrings to capture versions,
        // but default to semver parsing
        versionTemplate: "^{{major}}.{{minor}}.0"
    }
}

Would probably break down on edge cases though since it requires an exact peer version and specific versioning scheme 😅

@renovatebot renovatebot locked and limited conversation to collaborators May 9, 2023
@rarkins rarkins converted this issue into discussion #22040 May 9, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
help wanted Help is needed or welcomed on this issue manager:npm package.json files (npm/yarn/pnpm) priority-4-low Low priority, unlikely to be done unless it becomes important to more people type:feature Feature (new functionality)
Projects
None yet
Development

No branches or pull requests

8 participants