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

fix: luke-cage preview graduated #119

Merged
merged 1 commit into from Sep 27, 2021

Conversation

@renovate
Copy link
Contributor

@renovate renovate bot commented Sep 27, 2021

WhiteSource Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
@octokit/plugin-rest-endpoint-methods 5.11.2 -> 5.11.3 age adoption passing confidence

Release Notes

octokit/plugin-rest-endpoint-methods.js

v5.11.3

Compare Source

Bug Fixes

Configuration

πŸ“… Schedule: At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

β™» Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

πŸ”• Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box.

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate renovate bot added the maintenance label Sep 27, 2021
@octokit-js-project-board octokit-js-project-board bot added this to Maintenance in JS Sep 27, 2021
@gr2m gr2m changed the title fix(deps): update dependency @octokit/plugin-rest-endpoint-methods to v5.11.3 fix: luke-cage preview graduated Sep 27, 2021
@gr2m gr2m enabled auto-merge (squash) Sep 27, 2021
gr2m
gr2m approved these changes Sep 27, 2021
@gr2m gr2m merged commit 38a823f into master Sep 27, 2021
8 checks passed
Loading
@gr2m gr2m deleted the renovate/octokit-plugin-rest-endpoint-methods-5.x branch Sep 27, 2021
JS automation moved this from Maintenance to Done Sep 27, 2021
@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Sep 27, 2021

πŸŽ‰ This PR is included in version 18.11.2 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 27, 2021

@gr2m Should this change have updated "@octokit/plugin-paginate-rest" to 2.16.4?

A simple install does not get the latest version, leading to:

node_modules/@octokit/plugin-paginate-rest/dist-types/generated/paginating-endpoints.d.ts:1351:31 - error TS2339: Property 'GET /user/{username}/packages' does not exist on type 'Endpoints'.

1351         parameters: Endpoints["GET /user/{username}/packages"]["parameters"];

Loading

@gr2m
Copy link
Member

@gr2m gr2m commented Sep 27, 2021

can you try to delete node_modules and your lockfile and install again? That should resolve the problem with GET /user/{username}/packages. It was a bad route coming from GitHub's OpenAPI spec, unfortunately it made it into a release of @octokit/plugin-paginate-rest, but it has been fixed meanwhile

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 27, 2021

The failure is in the github actions build so not sure I can clear that easily. Since the deps still says 2.16.0, it is not really fixed yet.

Loading

@gr2m
Copy link
Member

@gr2m gr2m commented Sep 28, 2021

The deps says ^2.16.0 so it includes all releases between 2.16.0 and <3.0.0. Delete the package-lock.json file and run npm install, then push the change to gitgitgadget/gitgitgadget#716

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 28, 2021

I could do that for this project but npm says 1600+ packages depend on this. If npm does not automatically update to the latest release, there could be many broken builds. I see in a previous issue this was being addressed but seems to have slipped through. I can push a change but thought you could probably do it faster.

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 28, 2021

Pushing a change from my end may not be that simple. The package-lock.json is still version 1. A (very) quick glance at npm install doc does not indicate the old version can be retained and I am not really interested in running old/multiple versions of npm.

Loading

@gr2m
Copy link
Member

@gr2m gr2m commented Sep 28, 2021

1600+ packages depend on this

these packages don't get your lock file, so they will be fine once they update.

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 28, 2021

these packages don't get your lock file, so they will be fine once they update.

Sorry, but I am confused. I understand they do not have my lock file. They have their own lock file that also specifies 2.16.0. I ran npm install and @octokit/plugin-paginate-rest was not updated from the current version so the compile error showed up. Unless projects are doing clean installs will they not have the same problem? The failing github action is modeled after others so they would have the same exposure. The current version of @octokit/rest requires v2.16.4 so it should not be left to npm to decide what version to use.

Loading

@gr2m
Copy link
Member

@gr2m gr2m commented Sep 28, 2021

npm will use the latest version within the given range. This is a common enough case to have a bad release. And the fix is just as common: delete your lock file and node_modules folder and run npm install.

If you are still unsure how it all works, can you reach out to npm's support or community? I don't have the time, and this is not an Octokit question, but an npm question.

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 28, 2021

npm install does not appear to use the latest version if the currently installed version is in the range. npm update will probably update to the latest version.

This is an Octokit question. It will NOT build with:

"@octokit/plugin-paginate-rest":  less than "2.16.4"

octokit/rest should not be assuming npm will update plugin-paginate-rest. Do you think it should?

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 28, 2021

I am not really interested in running old/multiple versions of npm.

Turns out to be pretty easy using a docker image. I have opened PR #120 for this. The lock file was already up to date.

Loading

@G-Rath
Copy link
Member

@G-Rath G-Rath commented Sep 28, 2021

And the fix is just as common: delete your lock file and node_modules folder and run npm install.

Alternatively you can install the specific package at the version as a dependency which'll have npm "update" that specific package since the version you're requesting fits our version constraint too so npm will deduplicate it - then you can remove the package as a direct dependency from your package.json, as npm won't downgrade the version.

Or you can remove that package from the lock - I've written a script for doing that because npm update is an "all or nothing" command in v6 (so I use that script at work for updating specific packages with security vulnerabilities - npm v7 iirc does let you pass specific packages to update). (For completeness, I also have one for yarn too)

Loading

@webstech
Copy link
Contributor

@webstech webstech commented Sep 29, 2021

And the fix is just as common: delete your lock file and node_modules folder and run npm install.

Alternatively you can install the specific package at the version as a dependency ...

Or you can remove that package from the lock ...

These are all tried and true methods but require manual work. Dependabot is not doing that. Could dependabot make it happen for the package being updated? Maybe. Recursively running update on the deps could work but why should it be needed?

Can tests be written on this repo to catch this edge case? Maybe.

Sorry I was a little aggressive on getting this fixed. This PR was not the one causing the problem and I should have seen that earlier.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
JS
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants