Pinning versions of dependent types

[wolfy1339]

As you may have noticed, the version of @octokit/openapi-types, @octokit/types, and the JS modules such as @octokit/plugin-rest-endpoint-methods are intertwined.

Since the versions of the type packages is only locked to a major version, if a bad release happens (and are still in the same major version) then all downstream users get the bad release when using the Octokit packages.

Of course, the same can happen in the same minor version.

I'm opening this discussion so I can gather feedback on the proper course of action that people feel comfortable with, and that would be a solution to this issue.

[wolfy1339]

One solution that has been brought up is to pin the dependencies.

Since we open a PR with each new release anyways, I don't personally feel it would hinder anything.

[wolfy1339]

@octokit/js @octokit/extensibility-sdks Can you provide your input

[gr2m]

several @octokit/* packages are dependands on the type packages. I'd be worried that we might get into situations where multiple versions of the type packages are loaded. It's not only a problem with the types, but also with the typescript language server performance. With the 700+ endpoints we are already bringing it down to its knees. If the server would need two or more versions of the types, then it will likely become unusable.

[oscard0m]

That's a very fair point Gregor. Do we know are facing this performance issue? Should we run some trace experiments here or ask for some guidance in the TS community?

@wolfy1339 To make sure I'm understanding:

Since the versions of the type packages is only locked to a major version, if a bad release happens (and are still in the same major version) then all downstream users get the bad release when using the Octokit packages.

Does this mean minor and patch versions of @octokit/types and @octokit/openapi-types are introducing breaking changes to our users?

[G-Rath]

I'm pretty against pinning dependencies because it makes people a lot more reliant on the package being actively maintained and that the developers act in the best interest of every person using that package (e.g. people have different thoughts about security, and if I'm using a package with a pinned dependency that has a security vulnerability which the maintainer decides is fine, that doesn't mean I don't suddenly have to not talk to pentesters, auditors, etc, and because its pinned rather than constrained my available options tend to be heavy).

Does this mean minor and patch versions of @octokit/types and @octokit/openapi-types are introducing breaking changes to our users?

Technically yes, but the nature of TypeScript as a superset of a language that does not require correct types to run means you can't really escape this without a lot of major version churn - that's why TypeScript & @types/ don't follow semver in the traditional sense.

We do still avoid unnecessary breaking changes in non-major versions (or at least non-patch, depending on the package), such as renaming interfaces.

[wolfy1339]

Does this mean minor and patch versions of @octokit/types and @octokit/openapi-types are introducing breaking changes to our users?

We try to respect semver for the most part, but some small updates can cause breaking changes. It's not intentional

[gr2m]

if a bad release happens

How often does it happen though? Can you point to examples? I don't think there is a perfect solution for this, everything is a trade-off. I'm trying to understand the impact of bad releases, and how we could maybe prevent them from happening in the first place

[wolfy1339]

The first one is a combination of npm releasing a breaking change without listing it anywhere, and Pika being unmaintained and archived.

See octokit/plugin-retry.js#405 for all the details

[G-Rath]

So the problem was that npm got updated, because we don't have a constraint on the version of npm we're using.

I think at this point @gr2m's original comment is the first thing we should be diving into - so far you've provided two examples, which while annoying, unless there are others or you have strong reason to expect this to happen very frequently, I don't know if they're worth the tradeoffs we'll probably have to make.

One thing I think it would make sense to do would be to set-up an E2E suite that runs say once a week, which could just do basic "brute forcing" of all these packages to confirm they can be installed, required/imported, and that typechecking doesn't error - that way we can feel comfortable that we should at least be able to catch these more exotic problems independently even if it happens after the release has gone out.

[oscard0m]

Isn't too late running an E2E weekly? Would it be possible to prepublish + E2E before any release?

[wolfy1339]

So the problem was that npm got updated, because we don't have a constraint on the version of npm we're using.

The version of npm used for releases comes from semantic-release

[G-Rath]

@oscard0m if there are tests that we can have running for specific packages before prepublish then yeah we should have them there, but what I'm talking about is a bigger thing, similar to eslint-remote-tester (which takes hours to run).

I don't think there's any gain in us trying to clone a whole bunch of source repos, but having such a package.json with literally every single one of our dependencies listed for install and then importing their relative files is not something we're going to be able to run for every release of every package (imo).

It would also be an iterative process - once we've got it setup, there might be aspects we find that can be easily integrated (and maintained!) in the CI for each individual package, and likewise we might find other "gotchas" that create these bad releases which we can add to this large-scale E2E tester.

Overall, the idea is it's a "just in case" for things like what @wolfy1339 has hit where the tradeoffs to ensure they don't happen on a specific package vs the likelihood of it happening again don't justify the cost of fixing them at that package level