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

Let wp-scripts packages-update install packages for a specific version of WordPress. #36005

Closed

Conversation

tomalec
Copy link
Contributor

@tomalec tomalec commented Oct 27, 2021

Description

This is to mitigate the problems related to DEWP usage mentioned in #35630.

This allows a plugin developer to install to-be-extracted dependencies at the lowest version of their plugin targets. To be able to run local tests and linters against that version. To give some insight to the developer, on what is the lowest anticipated version of an individual package.

How has this been tested?

  1. Cloned and installed a plugin, like git clone https://github.com/woocommerce/google-listings-and-ads.git && npm install
  2. Replaced the node_modules/@wordpress/scripts with the content of this branch
  3. Executed npm run packages-update / wp-scripts packages-update, to see it still installs the latest by default
  4. Executed npm run packages-update -- --wpVersion=5.6 / wp-scripts packages-update--wpVersion=5.6, to see it installs the versions from https://github.com/WordPress/wordpress-develop/blob/5.6/package.json#L79-L154

Screenshots

Types of changes

New feature

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • [n/a] I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • [n/a] I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

Additional notes:

…ion of WordPress.

This is to mitigate the problems related to DEWP usage mentioned in WordPress#35630.

This allows a plugin developer to install to-be-extracted dependencies at the lowest version of their plugin targets. To be able to run local tests and linters against that version. To give some insight to the developer, on what is the lowest anticipated version of an individual package.
@tomalec
Copy link
Contributor Author

tomalec commented Dec 1, 2021

@gziolo Would you find some time to take a look at this?
If you find this idea worth considering, I also thought about implementing another tiny feature:

@gziolo gziolo added [Tool] WP Scripts /packages/scripts [Type] New API New API to be used by plugin developers or package users. labels Dec 1, 2021
@gziolo
Copy link
Member

gziolo commented Dec 2, 2021

One observation for the proposed command:

wp-scripts packages-update --wpVersion=5.6

While this works perfectly fine outside of the https://github.com/WordPress/wordpress-develop repository, it might be unclear what is supposed to do when running the same command from https://github.com/WordPress/wordpress-develop. We primarily use wp-scripts packages-update in the WordPress core's trunk while updating packages to the latest version. Anyway, the proposed improvement looks interesting.

If I understand correctly the new flag is more a way to mirror an existing configuration in WordPress Core. So maybe we should reflect that in the name of the flag. I'm really tempted now to refine the Gutenberg plugin release so it also includes npm publishing so you could replicate the same thing for the plugin. It would be more complex as you would have to read the version from each individual package, but I think it's doable.

@tomalec
Copy link
Contributor Author

tomalec commented Dec 2, 2021

While this works perfectly fine outside of the WordPress/wordpress-develop repository, it might be unclear what is supposed to do when running the same command from WordPress/wordpress-develop. We primarily use wp-scripts packages-update in the WordPress core's trunk while updating packages to the latest version.

Thanks for pointing that, I didn't think about it before. I double-checked locally how it behaves.
I think wpVersion flag may not be very useful from wordpress-develop repo. But still, it will set (up-/downgrade) packages to the versions specified for the given WP version. IMHO, it's not confusing and quite expected. I imagine it could be useful when:

  • debugging, and just rolling version-by-version down to pin the bug
  • checking how the current git revision works with packages at given version, to prepare a hot fix or rollback

But, yes, the main purpose of this feature to me is for external plugins. In following scenarios:

  1. Setting up a repo, when we say we support WPa.b+ (while the latest is c.d), so we could use it for local development, and unit testing.
  2. Increasing this minimum version, when the new one is shipped, or when we decide to drop the legacy. So we could run all the tests on the slightly newer versions, and remove any workarounds, legacy glue code, we had to support the older one.
  3. Implementing more dense test coverage, when we would like the CI to test the matrix of versions, by do something like:
    npm install
    npm test
    wp-scripts packages-update --wpVersion=5.6
    npm test
    wp-scripts packages-update --wpVersion=5.7
    npm test
    
  4. Debugging/investigating/trying to reproduce a customer-reported bug To be able to quickly set-up an environment closet to the one used by the customer.

If I understand correctly the new flag is more a way to mirror an existing configuration in WordPress Core

I hope the above points explain a bit more my motivation and rationale behind the feature. In short, it's to make local packages (node_modules) reflect/mimic the setup available (through DEWP) on a WordPress x.y installation


So maybe we should reflect that in the name of the flag

Do you have something in mind?


I'm really tempted now to refine the Gutenberg plugin release so it also includes npm publishing so you could replicate the same thing for the plugin

👍 That would be awesome!

So we could do something like

wp-scripts packages-update --gbVersion=7.5.0

(somewhat related to dist-tags idea)

@tomalec
Copy link
Contributor Author

tomalec commented Dec 2, 2021

It would be more complex as you would have to read the version from each individual package, but I think it's doable.

I'll probably, need that anyway. For a plugin that is a WooCommerce extension, I'd need to inspect the individual packages.

Unless, Gutenberg or WooCommerce-admin would expose a map of their releases to individual versions, in some other form.
Like an asset generated to every (GH) release: depenencies.json, or package.json.

I think such a map could deliver value on its own. As would give a quick, and both machine- & human-readable source to check the connected versions.

But that would require as you already mentioned, a change in the release process.

@gziolo
Copy link
Member

gziolo commented Jan 13, 2022

Resharing my comment from #24376 (comment) where we discuss the proposal for using npm-dist-tag for plugin and core releases. It feels like when that happens we can simplify this proposal to allowing passing a dist-tag when running the command, example:

wp-scripts packages-update —dist-tag=gb-12.4

@tomalec
Copy link
Contributor Author

tomalec commented Jan 14, 2022

It feels like when that happens we can simplify this proposal to allowing passing a dist-tag when running the command, example:

I think so, but with few caveats:

  • dist-tag will work only for new packages that uses that
  • wpVersion, works today, and works for older WP releases
  • dist-tag would work only for Wordpress/Gutenberg controlled packages.
  • while wpVersion could fetch versions of packages governed by other parties - I plan to add another extension to wp-scripts packages-update to take .externalized.json from DEWP as a config, and update also other externalized packages, like lodash, react etc.

@gziolo
Copy link
Member

gziolo commented Jan 14, 2022

Right, it would work only for newly published WordPress packages, but it would be an issue only for a few months. We can always set dist-tags manually for older versions of WordPress based on the configuration of branches in https://github.com/WordPress/wordpress-develop.

@gziolo
Copy link
Member

gziolo commented Apr 21, 2022

Resharing my comment from #24376 (comment) where we discuss the proposal for using npm-dist-tag for plugin and core releases. It feels like when that happens we can simplify this proposal to allowing passing a dist-tag when running the command, example:

wp-scripts packages-update —dist-tag=gb-12.4

I opened an alternative PR that implements my comment from earlier: #40514. We need that to have a better workflow for WordPress 6.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] WP Scripts /packages/scripts [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants