Skip to content

fix: let Renovate link node_modules when updating lockfiles #10311

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.

Sign up for GitHub

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

Merged
cryptodev-2s merged 1 commit into from
Sep 21, 2026
+1 −3
Merged

fix: let Renovate link node_modules when updating lockfiles #10311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/renovate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,8 @@ jobs:

- name: Run Renovate
uses: renovatebot/github-action@dcfba84a42d1b5d5e49bf131b1bf53511851a123 # v46.3.1
with:
additional-env-list: COREPACK_ENABLE_DOWNLOAD_PROMPT
env:
RENOVATE_TOKEN: ${{ steps.get-token.outputs.token }}
RENOVATE_REPOSITORIES: ${{ github.repository }}
RENOVATE_PLATFORM: github
COREPACK_ENABLE_DOWNLOAD_PROMPT: '0'
LOG_LEVEL: ${{ inputs.log-level || 'info' }}
1 change: 1 addition & 0 deletions renovate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"dependencyDashboardApproval": true,
"minimumReleaseAge": "3 days",
"postUpdateOptions": ["yarnDedupeHighest"],
"skipInstalls": false,

@Mrtenz Mrtenz Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just disable the post-install for Renovate? There's no reason to run it.

Bit hacky, but this is how we do it in the extension:

https://github.com/MetaMask/metamask-extension/blob/4cc1e269a524007ed3f633d0399b48429b91be22/package.json#L41

@Mrtenz Mrtenz Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are we doing this yarn run postinstall then? 🤔

@cryptodev-2s cryptodev-2s Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're not, yarn is. it's a root lifecycle script so yarn runs it itself after the Link step, that's where the yarn run ... <scriptName> in the error comes from.

and it never gets to the script body anyway. swapped it for a bare node -e "console.log('hi')" and same failure.

@Mrtenz Mrtenz Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with yarn install --mode=update-lockfile, Yarn still runs postinstall, even though it isn't able to? That seems confusing.

@cryptodev-2s cryptodev-2s Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah you were right to doubt it, i had it wrong. yarn doesn't run postinstall in that mode at all.

it's the lavamoat allow-scripts plugin in .yarnrc.yml. its afterAllInstalled hook shells out to yarn run allow-scripts, which needs the node_modules state file update-lockfile never writes. dropping the postinstall changes nothing, dropping the plugin makes it pass.

same fix though, skip-build runs the Link step. PR description updated.

@Mrtenz Mrtenz Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that makes more sense.

Maybe we should add an environment variable to the allow-scripts plugin (or detect the install mode, if that's possible) so we can skip it when it's not needed instead?

I can approve this for now, but maybe look into that as follow-up.

@Gudahtt Gudahtt Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add an environment variable to the allow-scripts plugin (or detect the install mode, if that's possible) so we can skip it when it's not needed instead?

I had the same problem when using Renovate on my Prettier plugin, and this is the same idea I landed on 😄. I forgot to mention it, and haven't implemented it yet. Very easy to do though.

"packageRules": [
{
"description": "Mirrors npmPreapprovedPackages in .yarnrc.yml, which exempts these from the install-time age gate.",
Expand Down
Loading