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

Replace unreliable activation hook with default value for enabled modules #222

Merged
merged 5 commits into from
Mar 14, 2022

Conversation

felixarntz
Copy link
Member

Summary

Fixes #218
Fixes #219

Relevant technical choices

  • The usage of plugin activation hooks is universally problematic when it comes to environments like multisite or tools like WP-CLI, since the hook may not get triggered, or in multisite it would only be triggered on the main site when network-activating the plugin.
  • Therefore this PR replaces it with using a default value for the option.
    • The straightforward implementation here would be to either hard-code the list of non-experimental modules in there or to dynamically read them from the filesystem, however both of that is not desirable: Hard-coding would result in manual maintenance which would be easily overlooked, and dynamically reading from the filesystem would be bad for performance since this would then need to run on every page load, even in the frontend of the site.
    • Because of the above, this PR uses a separate PHP file default-enabled-modules.php which, similarly to the existing module-i18n.php file, can be automatically generated and maintained.
  • For the above, the PR implements a new CLI script similar to some of the existing ones, which allows to refresh the new auto-generated PHP file with a single command.
  • The release instructions are being expanded accordingly to include running this new command before every plugin release.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@felixarntz felixarntz added [Type] Bug An existing feature is broken Infrastructure Issues for the overall performance plugin infrastructure Needs Review labels Mar 8, 2022
@felixarntz felixarntz added this to the 1.0.0-beta.2 milestone Mar 8, 2022
@felixarntz felixarntz changed the title Replace unreliable activation hook with default value for module setting Replace unreliable activation hook with default value for enabled modules Mar 8, 2022
@gagan0123
Copy link

Tested scenarios,

  1. Multisite
    1. Activate on single site
    2. Network Activate
  2. WP-CLI install and activate in single and multisite
  3. WP-CLI activate in single site

All scenarios now working as expected with default values as they should be.

bin/plugin/commands/common.js Outdated Show resolved Hide resolved
load.php Show resolved Hide resolved
@felixarntz felixarntz requested a review from mitogh March 9, 2022 19:47
@felixarntz felixarntz merged commit 0cad854 into trunk Mar 14, 2022
@felixarntz felixarntz deleted the fix/reliable-default-enabled-modules branch April 15, 2022 18:22
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 [Type] Bug An existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default settings not working with WP-CLI Default settings not working as expected in multisite
4 participants