WIP PHP Plugin scaffolding

[JPry]

PHP Plugin Scaffolding

This PR implements an initial framework to continue building out this plugin. I'll cover that approach I'm using, my ideas for the long term, as well as what is needed as next steps.

Approach

I had in mind to utilize as much modern PHP practice as possible. For inspiration, I tuned to some of the recent work done in WooCommere core (dependency injection), plus a framework developed by others in the WordPress community (Alain Schlesser's speaking page plugin. There are a few key concepts that I feel are important:

• The code should be modular. Each class should have its own specific purpose, and it should not do too much.
• Common code should be moved to traits and/or abstract classes where appropriate. This allows for keeping out code DRY, which will help with maintenance in the future.
• Interfaces should be used so that classes can declare what the need to do. This allows for calling code to handle each class as needed with minimal "wiring" of the class
• We should take advantage of autoloading and dependency injection. If a class needs to depend on another class for functionality, it should accept that dependency in the constructor.
• The main plugin file should simply be an entry point where our code can be kicked off. But it should not be the only entry point. That will allow for proper unit testing independent of WordPress and WooCommerce
• We should avoid singletons and static instance() methods. As much as possible, ensuring that classes be instantiated only once should be handled outside of each class.
• When we need to add new functionality, it should require minimal code changes. Ideally, it should be a matter of:
  1. Create a new class
  2. Register the class in our container
  3. Watch it work!

With these points in mind, I've set up an initial version of this plugin. The main plugin file calls a factory class to kick off the main plugin on the woocommerce_loaded hook. We make use of containers and services, as modeled in WooCommerce core, to provide our functionality to the plugin.

Any kind of asset handling (such as scripts and styles) outside of PHP has a bit of a special case. Essentially, each individual asset should be instantiated with a class that implements the \Automattic\WooCommerce\GoogleForWC\Assets\Asset interface, and enqueuing should be mostly automatic.

Long Term Ideas

New functionality should be separated into components that make sense. I've tried to start that process by creating the \Automattic\WooCommerce\GoogleForWC\Menu and \Automattic\WooCommerce\GoogleForWC\Pages namespaces. We'll likely need other top-level components like RestAPI and some others. This structure can be determined by us all as we work on the project to figure out what makes the most sense.

Next Steps

Right now, I've got an initial version of this working. Here are some of the next steps:

• We need to create a StyleAsset for handling our style registration.
• Because many of our JS components have a build process that will generate a PHP dependency file, we should have some special handling for these components. Probably a different extension of the ScriptAsset class
• Our current container implementation is building on top of WooCommerce's implementation. One gotcha I ran into is that WC has a custom \Automattic\WooCommerce\Internal\DependencyManagement\Definition class. This is specially built for legacy code in WC core that relies on a static instance() method. We should be careful to avoid using that class and instead use the original Definition class. Aside from that issue, I don't feel very happy with how it's currently working, so I'm very much open to ideas for improvement there.
• We will need to have some admin notices in place for various situations that might occur when the plugin is activated and we can't properly run (e.g. WC not active, too-old version of PHP, etc.)
• We'll need some unit testing put in place. I had in mind that we could try to aim for true unit tests (testing individual pieces of functionality) plus integration tests that ensure proper E2E testing.

Plus all of the things still missing from the original issue.

[JPry]

@layoutd @jconroy I've got all of my in-progress stuff pushed here, and I've updated the description with more details about where this stands. Feel free to ping me with requests for more info or any questions.

[jconroy]

Just noting that I needed to switch to woocommerce master to avoid an error.

Looks like the Dependency Injection container work was only merged recently woocommerce/woocommerce#27733

So I think we would need WC 4.7+ place which is in RC

Also, I think some of the php syntax being used might be 7.1+ and we have a 7.0 minimum for Woo core. I've added some notes to the main readme under prerequisites

[jconroy]

Not 100% sure why but I need to delete the updated package-json.lock file on this branch and start fresh to be able to run the build properly.

[jconroy]

@ecgan can you try out this branch (you will need WooCommerce 4.7-rc1 +) and see if you have the same problem with package-lock.json

[ecgan]

@jconroy ,

Not 100% sure why but I need to delete the updated package-json.lock file on this branch and start fresh to be able to run the build properly.

Could you provide more details? Do you see an error when running build?

I checkout the branch and run the usual composer install, npm install and then npm run build, and I see the following error:

> google-for-woocommerce@0.1.0 build /Users/engchin/Code/woocommerce/google-listings-and-ads
> NODE_ENV=production wp-scripts build

/Users/engchin/Code/woocommerce/google-listings-and-ads/node_modules/ignore-emit-webpack-plugin/index.js:64
                compilation.hooks.processAssets.tap({
                                                ^

TypeError: Cannot read property 'tap' of undefined
    at /Users/engchin/Code/woocommerce/google-listings-and-ads/node_modules/ignore-emit-webpack-plugin/index.js:64:49
    at SyncHook.eval [as call] (eval at create (/Users/engchin/Code/woocommerce/google-listings-and-ads/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:9:1)
    at SyncHook.lazyCompileHook (/Users/engchin/Code/woocommerce/google-listings-and-ads/node_modules/tapable/lib/Hook.js:154:20)
    at Compiler.newCompilation (/Users/engchin/Code/woocommerce/google-listings-and-ads/node_modules/webpack/lib/Compiler.js:631:26)
    at hooks.beforeCompile.callAsync.err (/Users/engchin/Code/woocommerce/google-listings-and-ads/node_modules/webpack/lib/Compiler.js:667:29)
[...redacted]

The above has been (temporarily) fixed in a PR for wp-scripts, WordPress/gutenberg#26591.

In my local, I merge the trunk branch into this feature/scaffolding branch, and then re-run composer install, npm install and then npm run build, and it all works okay.

(you will need WooCommerce 4.7-rc1 +)

Actually, why do I need the WooCommerce core plugin? 🤔 I did the above process without the plugin.

[jconroy]

That’s the same error - thanks - just wanted to confirm you were seeing the same.

Thats interesting about merging in trunk - must be something in the dependencies somewhere

why do I need WooCommerce

You don’t to run npm - my comment was just in relation what you’ll need to run this branch (to be able to install the plugin etc)

@ecgan

[ecgan]

@jconroy ,

Thats interesting about merging in trunk - must be something in the dependencies somewhere

Yes indeed, it's with the @wordpress/scripts, the merge I did bumps "@wordpress/scripts": "^12.2.1" to "@wordpress/scripts": "^12.5.0".