search

octokit / plugin-rest-endpoint-methods.js

Octokit plugin adding one method for all of api.github.com REST API endpoints
MIT License
117 stars 56 forks source link

[10.x] fix(deps): upgrade `@octokit/types` to v13 #741

Closed Bo98 closed 6 months ago

Bo98 commented 6 months ago

Before the change?

@octokit/plugin-rest-endpoint-methods v10 is the final version to support @octokit/core v5.

@octokit/core v5 recently got a 5.1.1 release that bumped @octokit/types to v13: https://github.com/octokit/core.js/releases/tag/v5.1.1.

@octokit/plugin-rest-endpoint-methods v10 was however still on @octokit/types v12, causing npm to install multiple copies in nested node_modules to resolve the version conflict.

After the change?

The version of @octokit/types is aligned with the latest @octokit/core v5, which means only one @octokit/types version needs to be installed.

Pull request checklist

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

github-actions[bot] commented 6 months ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

wolfy1339 commented 6 months ago

This plugin is trickier, since there were some breaking changes in the REST API.

It wouldn't make sense to put these breaking changes in a patch release

Bo98 commented 6 months ago

Hmm ok, make sense - thanks for the clarification.

The confusion was if you had @octokit/core v5 and then added this plugin later, then npm would downgrade to v12 (moving v13 to be a internal copy of @octokit/core and friends) which could break some direct @octokit/types usage that relied on v13.

But it seems like this might be impossible to fix unfortunately.

wolfy1339 commented 6 months ago

It could be possible to fork the current version v13 and revert the changes that made it incompatible with earlier versions of @octokit/core in order to make a final version.

Bo98 commented 6 months ago

That would make sense yeah. There'll probably be some @octokit/core v5 usage for a while given @actions/github etc are sometimes slow to update.

To add to my earlier comment, what npm does seems very indeterminate actually (deleting the lockfile seems to flip the default compared to an npm install update?) - but I guess that's more a bug on my side for failing to depend on @octokit/types directly.

If you want/need this PR open again let me know.

wolfy1339 commented 6 months ago

Depending on @octokit/types directly is hard, because it needs to stay in sync with the version used across the Octokit packages (up to the latest minor version)

wolfy1339 commented 6 months ago

https://github.com/octokit/plugin-rest-endpoint-methods.js/releases/tag/v13.2.2

Bo98 commented 6 months ago

Oh right v13 of this repo - thought you meant reverting the removals/changes from @octokit/types v13 initially.

Makes sense though note that there is plugin-paginate-rest that's the same deal that may need updating similarly - I just hadn't opened a PR there.

wolfy1339 commented 6 months ago

https://github.com/octokit/plugin-paginate-rest.js/releases/tag/v11.3.1

Bo98 commented 6 months ago

Thank you!