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

Implement CLI command to for a build process to transform modules into standalone plugins #635

Closed
Tracked by #656
felixarntz opened this issue Feb 1, 2023 · 8 comments · Fixed by #662
Closed
Tracked by #656
Assignees
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Feb 1, 2023

Feature Description

Following #618 (comment), we now have an initial path forward for unbundling the Performance Lab plugin and creating standalone plugins from modules. While it is still being discussed whether we will publish all modules as standalone plugins or just a subset of them, we can definitely start working on the infrastructure changes needed.

For the purpose of this issue, let's simply start with one module, images/webp-uploads. This is just so that we can get started with one plugin. database/sqlite would be the other big priority to publish as standalone plugin, but that one is already available through the separate repo, so it's not a good idea to cover it here. In fact, it should be handle in a separate issue as bringing the codebase back into this monorepo will have its own little quirks to consider.

Requirements

  • Add a config file (e.g. something like plugins.json) which contains a map of module directories and their associated standalone plugin configuration (for now probably just the WordPress.org plugin repository slug and intended version). The file should for now only contain the image/webp-uploads module. For example:
{
  "images/webp-uploads": {
    "slug": "webp-uploads",
    "version": "1.0.0"
  }
}
  • Implement a new Node CLI command build-plugins in bin/commands that does the following:
    • Copy over all module directories based on the above config file (i.e. for now only image/webp-uploads) into a (git-ignored) build directory.
    • After copying over, replace each load.php file's module header with a corresponding plugin header:
      • Plugin Name and Plugin Description should use the module name and description respectively.
      • Plugin URI should point to the module's directory in this GitHub repository.
      • Text Domain should match the target plugin's slug.
      • Version should use the target version from the config file.
      • All other fields (requirements, author, license, ...) should match what is in the Performance Lab plugin's own plugin header.
    • Replace all usages of the 'performance-lab' text domain with the correct text domain for the individual plugin based on its slug (see above).
@joemcgill
Copy link
Member

joemcgill commented Feb 2, 2023

@felixarntz I think these requirements for a build process look pretty good to me. The main thing I think we should add here is that each built plugin should also include a README file (covered in #636, but should be thought of together with this task, I think). I think it would be easiest if we were to add module-specific readme's in folders with each module. Having a folder for each module would also allow us to add independent configuration files in each that would responsible for things like text-domains, plugin slugs, etc. if we wanted to keep those independent from a single plugins.js file as you're proposing here.

@felixarntz
Copy link
Member Author

@joemcgill I think we can handle #635 and #636 separately. I'd prefer to iterate and we don't have to do all of that in one PR. Coming up with the readme content itself will be a bit of extra work, and that IMO shouldn't block us from working on this issue. #636 would include expanding the build process implemented in this issue here accordingly, to build the readme.txt files as well.

The changes proposed in this issue here can already work by themselves, so I don't see why we would need to combine both issues.

I think it would be easiest if we were to add module-specific readme's in folders with each module.

Agreed, we already have the module directories so we can just add the readme files to them. Though per my above reply I think we should work on that as part of #636.

Having a folder for each module would also allow us to add independent configuration files in each that would responsible for things like text-domains, plugin slugs, etc. if we wanted to keep those independent from a single plugins.js file as you're proposing here.

To me, a single plugins.js file is easier to manage, and it also provides a single point to see which modules are configured to be published as standalone. All we need to provide in this file is the map of module directory and plugin slug, so I don't think that justifies individual files for each module.

@mukeshpanchal27
Copy link
Member

@felixarntz one question Do we need to use any Assets for the standalone plugin or not?

cc. @joemcgill

@mukeshpanchal27
Copy link
Member

Additional question: Do we need can-load.php file in standalone plugin?

@felixarntz
Copy link
Member Author

felixarntz commented Feb 27, 2023

@mukeshpanchal27

one question Do we need to use any Assets for the standalone plugin or not?

Great question. I think for now we don't have any, so let's not worry about it for now. We can follow up separately on that to see whether there's a chance for us to get some good icons / banner for those standalone plugins.

Additional question: Do we need can-load.php file in standalone plugin?

That's another great question. I think we should keep it for now in both, because it is critical even for the plugin that they don't run their functionality / add their hooks when those conditions from the can-load.php file are not satisfied.

For the PL plugin, the check is run prior to activation, which of course for a plugin doesn't work. But in the plugin it would need to happen when the plugin is already activated. I think we should add something like the following to the top of every module's load.php file that has a can-load.php file and should be made available standalone:

function moduleprefix_can_load() {
	$can_load = require __DIR__ . '/can-load.php';
	if ( $can_load() ) {
		return true;
	}
	add_action(
		'admin_notices',
		function() {
			// Render an error notice that the plugin cannot be initialized because of the conditions not being met.
		}
	);
	return false;
}

// Do not run the plugin if conditions are not met.
if ( ! moduleprefix_can_load() ) {
	return;
}

@mukeshpanchal27 mukeshpanchal27 self-assigned this Feb 28, 2023
@mukeshpanchal27
Copy link
Member

@felixarntz Some functions have two @since versions for updates. Do we need to remove the second?

https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L536-L537

cc. @joemcgill

@joemcgill
Copy link
Member

@mukeshpanchal27 In many cases, a second @since tag is added to note when an important change was made to the function, which seems to be the case here. Leaving it as is should be fine.

@mukeshpanchal27
Copy link
Member

closed this as completed in #662

@felixarntz felixarntz added the [Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only label Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants