Skip to content

Conversation

@desrosj
Copy link
Member

@desrosj desrosj commented Jan 28, 2025

Dependabot updates have proven useful for tracking and updating 3rd-party GitHub Actions.

Without these updates configured, someone needs to notice there is a new release for a dependency, create a PR, create a Trac ticket, etc.

Previously, we have avoided managing npm dependencies through Dependabot for fear of too much noise. However with 52 devDependencies and 24 non-@wordpress/ dependencies (which are managed through the grunt sync-gutenberg-packages script), this is not sustainable or a good use of time when it can be mostly automated.

To test this PR, I've gone and pushed this dependabot.yml file to my desrosj/wordpress-develop fork, enabled Dependabot updates, and downgraded every dependency in order to trigger an update PR. You can find the alerts in the open PR list.

I've organized all dependencies into groups of related functionality.

There are a few considerations though:

  • The Action workflows for some of the groups will almost always fail without contributor interaction in the current state. This is due to one of two things:
    • The built script file that must have committed.
    • The need to bump the version specified for the script in script-loader.php.
  • ` The need to change the expected version of a dependency in a test file.

For now, I think this is OK because someone can pick up the PR as a starting point and make the necessary final changes. In the future, we could add a GHA that detects when a built script file needs to be updated and automatically commit the difference for review. But once changes are made to a PR, Dependabot ignores it going forward. So that would need to be considered carefully.

Trac ticket: https://core.trac.wordpress.org/ticket/62221


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@desrosj desrosj self-assigned this Jan 28, 2025
@github-actions
Copy link

github-actions bot commented Jan 28, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props desrosj, swissspidy, joemcgill.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@desrosj
Copy link
Member Author

desrosj commented Jan 29, 2025

I've created a spreadsheet with all npm dependencies in order to double check the groupings. Everything is grouping as expected.

@desrosj
Copy link
Member Author

desrosj commented Jan 29, 2025

Also, ignore the Composer group in the dependabot.yml file. This was to test a group for #8137.

@swissspidy
Copy link
Member

Thanks for the thorough work looking into this!

Definitely a good opportunity to think about how to automate the rest too, like script-loader.php changes. Is there for example a Grunt script that updates the version based on package.json?

But once changes are made to a PR, Dependabot ignores it going forward. So that would need to be considered carefully.

Note that you can add [dependabot skip] to a commit message to continue having Dependabot rebase PRs. See https://docs.github.com/en/code-security/dependabot/working-with-dependabot/managing-pull-requests-for-dependency-updates#allowing-dependabot-to-rebase-and-force-push-over-extra-commits

@desrosj
Copy link
Member Author

desrosj commented Jan 30, 2025

Definitely a good opportunity to think about how to automate the rest too, like script-loader.php changes. Is there for example a Grunt script that updates the version based on package.json?

There are several replace tasks configured for updating or replacing strings in PHP files. The tricky part is that the package names don't always map exactly to the script handle. For example, the polyfill-library dependency is used to build wp-polyfill-dom-rect and wp-polyfill-node-contains.

Note that you can add [dependabot skip] to a commit message to continue having Dependabot rebase PRs. See https://docs.github.com/en/code-security/dependabot/working-with-dependabot/managing-pull-requests-for-dependency-updates#allowing-dependabot-to-rebase-and-force-push-over-extra-commits

TIL! This would still be easy to forget. So worth keeping in mind when considering the contributor flow here. I think automation is the better option anyway.

@desrosj
Copy link
Member Author

desrosj commented Feb 5, 2025

I've opened #8260 to address some of the changes to built files required as a result of updating dependencies. It does not change the script versions in the unit tests or script-loader.php, but is a start.

@desrosj
Copy link
Member Author

desrosj commented Feb 7, 2025

I'm currently leaning towards #8260 combined with monitoring devDependencies only in dependabot.yml as a first iteration. That way it won't create PRs that will always fail, but it allows some dependencies to be tracked more effectively.

@desrosj desrosj requested a review from joemcgill February 7, 2025 17:49
@joemcgill
Copy link
Member

We should definitely try to automate this as much as possible, so a big +1 from me. #8260 does look like a good start here. We should also consider ways that we could eliminate some of these manual tasks (rather than automating). Do you have an example of a test file that expects a specific dependency version to be defined?

@desrosj
Copy link
Member Author

desrosj commented Feb 17, 2025

We should also consider ways that we could eliminate some of these manual tasks (rather than automating).

Definitely agree the fewer tasks the better. Removing the script-loader-packages.php file from version control specifically came up in Trac-50460, but there has not been any traction on that yet.

Do you have an example of a test file that expects a specific dependency version to be defined?

Yes, the test_vendor_script_versions_registered_manually() test method will fail when script-loader.php is not updated to correctly reflect the right dependency version.

@desrosj desrosj added the props-bot Adding this label triggers the Props Bot workflow for a PR. label Mar 21, 2025
@github-actions github-actions bot removed the props-bot Adding this label triggers the Props Bot workflow for a PR. label Mar 21, 2025
@desrosj
Copy link
Member Author

desrosj commented Mar 21, 2025

@desrosj desrosj closed this Mar 21, 2025
@desrosj desrosj deleted the add/npm-dependabot branch March 21, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants