Reduce boilerplate and embrace default config #4
Conversation
| * | ||
| * @param string $build_path Absolute path to the build directory for all assets. | ||
| */ | ||
| public function __construct( string $build_path ) { |
There was a problem hiding this comment.
This now receives the path of the build directory via constructor argument. It is now the responsibility of the caller of this class (plugin bootstrap) to define where that directory lives (which also gives us a single place to update it).
| * Enqueues the block assets for the editor. | ||
| */ | ||
| public function enqueue_block_assets() { | ||
| $asset_meta = $this->build_dir->get_asset_meta( 'editor-script.js' ); |
There was a problem hiding this comment.
The Plugin_Paths now has a helper for resolving the *.asset.php meta file and returning the data, if the file is present. Removes a lot of repeated boilerplate.
| */ | ||
| public function get_url( ?string $relative_path = null ): string { | ||
| if ( isset( $relative_path ) ) { | ||
| return plugins_url( ltrim( $relative_path, '/' ), $this->plugin_dir . '/fake.file' ); |
There was a problem hiding this comment.
That /fake.file definitely feels like a hack but I'm not aware of a better helper that would accept a directory path as the argument :)
| * @return array|null Array with 'dependencies' and 'version' keys, or null if not found. | ||
| */ | ||
| public function get_asset_meta( string $asset_file ): ?array { | ||
| $asset_path = $this->get_path( dirname( $asset_file ) . '/' . pathinfo( $asset_file, PATHINFO_FILENAME ) . '.asset.php' ); |
There was a problem hiding this comment.
This is the new helper that resolves *.asset.php meta data for any JS/CSS file.
| wp_trigger_error( 'Advanced Multi Block Plugin: Composer autoload file not found. Please run `composer install`.', E_USER_ERROR ); | ||
| return; | ||
| // Include our bundled autoload if not loaded globally. | ||
| if ( ! class_exists( Advanced_Multi_Block\Plugin_Paths::class ) && file_exists( __DIR__ . '/vendor/autoload.php' ) ) { |
There was a problem hiding this comment.
Here we attempt to require the bundled vendor/autoload.php only if the class is not resolved by the global project autoloader. By default, calling class_exists() triggers the autoload logic so that attempts everything that a global Composer autoload has registered (if at all).
| <exclude-pattern>/build/</exclude-pattern> | ||
| <exclude-pattern>/node_modules/</exclude-pattern> | ||
| <exclude-pattern>/vendor/</exclude-pattern> | ||
| <exclude-pattern>src/blocks-manifest.php</exclude-pattern> |
There was a problem hiding this comment.
This was non-existent. The file is created under /build/blocks-manifest.php which is already excluded above.
| <exclude-pattern>/node_modules/</exclude-pattern> | ||
| <exclude-pattern>/vendor/</exclude-pattern> | ||
| <exclude-pattern>src/blocks-manifest.php</exclude-pattern> | ||
| <exclude-pattern>build/*.asset.php</exclude-pattern> |
There was a problem hiding this comment.
This is also already excluded via /build/ rule above.
| <config name="minimum_supported_wp_version" value="6.6"/> | ||
|
|
||
| <rule ref="WordPress"> | ||
| <exclude name="Generic.Arrays.DisallowShortArraySyntax"/> |
There was a problem hiding this comment.
Remove these opinionated excludes.
| <rule ref="WordPress.NamingConventions.PrefixAllGlobals"> | ||
| <properties> | ||
| <property name="prefixes" type="array"> | ||
| <element value="advanced_multi_block"/> |
There was a problem hiding this comment.
This is a nice feature. I've updated all the instances where this wasn't used.
| <rule ref="WordPress.WP.I18n"> | ||
| <properties> | ||
| <property name="text_domain" type="array"> | ||
| <element value="advanced-multi-block"/> |
There was a problem hiding this comment.
Today I learned that the WordPress ruleset doesn't extract this from the plugin meta header automatically which I assumed it would.
|
Thanks for the feedback and suggest @kasparsd, much appreciated, and apologies for the delay in making this comment. I just wrapped up my time planning WordCamp Canada and am planning on cycling back on this article and writing a revised article instead of updating the existing one. Updating the original would make the comments seem out of place, and I like that fact that it shows growth through collaboration. If you're interested, I'd love to have you as a reviewer on the revised article. Let me know if that's something you'd be interested in and I'll let you know when that's ready to check out. |
2 participants
As mentioned in the comment on the article:
Fix autoloader for projects where the plugin is added via composer -- include the local
vendor/autoload.phponly if a plugin class isn't resolved by the global autoloader.Remove all the extra configuration files for eslint and stylelint. In my experience they quickly drift/conflict from the bundled dependencies of
@wordpress/scriptscausing significant maintenance hurdles. It might be easier to rely on defaults since that is "zero-config" and works great.Introduce
Plugin_Moduleabstract class which helps avoid adding WP hooks in class__construct().Make classes agnostic to their location within the plugin. Pass configuration such as
buildpath for assets as a dependency.Use
npm-run-allpackage (similar to Gutenberg) to ensure that all npmscriptswork across different operating systems.Move PHP classes to
classesdirectory and rename files to match the WPCS rules.Fix all reported WPCS issues.
Please see the comments inline for context.