Dependabot update grouping does not update all npm packages in `package.json`

romainmenke commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Package ecosystem

npm

Package manager version

latest

Language version

latest

Manifest location and content before the Dependabot update

package.json before : https://github.com/stylelint/stylelint/blob/2c45589a4b730b2c7c0d2ffd75815ad3c9ac963f/package.json#L134-L136

package-lock.json before : https://github.com/stylelint/stylelint/blob/2c45589a4b730b2c7c0d2ffd75815ad3c9ac963f/package-lock.json#L1221-L1232

package.json after : https://github.com/stylelint/stylelint/blob/8ca025e55178d16c1786ce99c4d596c2ad99cb64/package.json#L134-L136

package-lock.json after : https://github.com/stylelint/stylelint/blob/8ca025e55178d16c1786ce99c4d596c2ad99cb64/package-lock.json#L1221-L1238

dependabot.yml content

https://github.com/stylelint/stylelint/blob/2c45589a4b730b2c7c0d2ffd75815ad3c9ac963f/.github/dependabot.yml#L1-L32

Updated dependency

"@csstools/css-parser-algorithms": "^2.3.1",
"@csstools/css-tokenizer": "^2.2.0",
"@csstools/media-query-list-parser": "^2.1.3",

What you expected to see, versus what you actually saw

I expected two things to be different :

I expected all three dependencies to be updated in package.json
I expected all three dependencies to be mentioned in the PR comment

Neither was true.

@csstools/css-tokenizer was only updated in package-lock.json and this change wasn't mentioned in the PR comment

In this specific case the real change was in @csstools/css-tokenizer as this package had a minor version bump with new features.

It was this change that should have been carefully reviewed as it is a direct dependency.

I suspect this is what happened :

@csstools/css-parser-algorithms comes before @csstools/css-tokenizer
Dependabot updates in order of appearance or 🔤
after updating @csstools/css-parser-algorithms the lock file already contained @csstools/css-tokenizer version 2.2.0 (it is a peer dependency)
because @csstools/css-tokenizer is already up to date in the lock file, it was skipped
because it was skipped it was not included in the PR comment

A possible fix would be to first build a dependency graph and then execute updates in topological order instead.

Then @csstools/css-tokenizer would have been updated first.

Native package manager behavior

N/A

Images of the diff or a link to the PR, issue, or logs

https://github.com/stylelint/stylelint/pull/7109/commits/8ca025e55178d16c1786ce99c4d596c2ad99cb64

Smallest manifest that reproduces the issue

No response

romainmenke commented 1 year ago

Just a friendly reminder that this issue is still present.

We currently work around it but it takes away a bit of the magic of dependabot grouped updates :)

romainmenke commented 11 months ago

This is still an issue :)

abdulapopoola commented 7 months ago

Hi @romainmenke ; does this still occur? The crew has shipped quite a few improvements in recent weeks.

romainmenke commented 7 months ago

This was still an issue very recently : https://github.com/stylelint/stylelint/pull/7560

Nishnha commented 6 months ago

Hi it looks like all of the dependencies were correctly grouped and updated in https://github.com/stylelint/stylelint/pull/7560/files

And the info looks correct in the PR body too.

Is the issue still present? @romainmenke

romainmenke commented 6 months ago

It is still present :)

I fix it manually : https://github.com/stylelint/stylelint/pull/7560/commits/298cc9ed479c24db3ca0a47dcc399f89576ed165

The PR comment body has indeed improved! I missed that.

So it is partially resolved?

Nishnha commented 6 months ago

Ah thanks I missed that. Will take a look!

Nishnha commented 6 months ago

I pulled the peer dependency relationship between these packages

css-tokenizer has no peer dependencies
css-parser-algorithms has a peer dependency on css-tokenizer
media-query-list-parser has a peer dependency on both css-tokenizer and css-parser-algorithms

I also made a minimal repro of the issue with just the 3 dependencies

"dependencies": {
  "@csstools/css-parser-algorithms": "^2.3.0",
  "@csstools/css-tokenizer": "^2.1.1",
  "@csstools/media-query-list-parser": "^2.1.2"
}

I'll step through our Dependabot CLI and see if there's a way to improve peer dependency updates!

Nishnha commented 6 months ago

I found that the #updated_requirements method doesn't run for the peer dependency updates. I'm not sure how the peer dependency is getting updated in the first place though, because @csstools/css-parser-algorithms updates with requirements_to_unlock: :own

ferdlestier commented 4 months ago

Hi all, just checking in on where we stand with this issue. Thanks in advance!

dependabot / dependabot-core