Modular product types

[joshuatf]

The usage of templates in the new product editor and attempt at combining features from multiple product types into a single template have exposed some long-standing challenges in WooCommerce product types.

I previously explored the option of deprecating product types in favor of the concept of "traits." I've since opened a few more concepts to explore this option to see what problems we might face in this process. My latest concept moves towards the ideas of "modules" in #45722 as pieces of functionaity that enhance the base product.

Relevant POC PRs:

#44657
#44585
#45635
#45722

Why modules

There a few major benefits to moving away from product types:

1. We allow a combination of multiple product features from different types within a single product. This can also be retroactively updated to declare compatibility or incompatibility.
2. Product templates (both in form and potentially frontend) become easier to manage as the compatibility engine handles turning on or off features within the template. The template is purely for restructuring the product blocks, if necessary.
3. We get a more consistent interface returned from the product factory. Instead of a class that may contain certain methods and not others, we always have the same set of methods, while product modules can be retrieved and cast as necessary.

The strategy

In the latest PR, #45722, I explore the idea of creating a new product type WC_Product_Modular that acts as the new base product extensible with modules. By creating a new product type, we:

1. Make this an opt-in feature and only expose it in the new editor.
2. Allow the existing product type classes to work as is without breaking existing code.
3. Let users convert a product back to one of the existing core product types if a compatibilty issue with an extension is found.

Implementation

This implementation keeps modules isolated and more closely follows SOLID principles. While earlier concepts explored integrating traits directly into the product class, we ultimately end up with an inconsistent interface depending on which modules are available. With this concept modules get registered on the woocommerce_product_modules hook:

$product_modules = apply_filters( 'woocommerce_product_modules', $product_modules );

foreach ( $product_modules as $module ) {
	if ( is_string( $module ) && class_exists( $module ) ) {
		$module::init();
		$this->product_modules[ $module::get_slug() ] = $module;
	}
}

Module instances get created on each product:

class WC_Product_Modular extends WC_Product {
	public function __construct( $product = 0 ) {
		parent::__construct( $product );
		$modules          = WC()->product_modules()->get_all_modules();
		$this->data_store = WC_Data_Store::load( 'product-modular' );

		foreach ( $modules as $module_class_name ) {
            		$module                        = new $module_class_name( $this );
            		$module_slug                   = $module::get_slug();
            		$this->modules[ $module_slug ] = $module;
		}
	}
}

Modules then can be activated, accessed, or deactivated:

$product = new WC_Product_Modular();
$product->activate_module( 'variable' );
$product->activate_module( 'subscription' );
$product->get_module( 'subscriptions' )->get_trial_length();
$product->get_module( 'variable' )->get_available_variations();
$product->deactivate_module( 'subscription' );

Next steps

• Get input from @woocommerce/proton and @woocommerce/mothra to get some buy-in to this idea.
• Flush out any blockers, issues with backwards compatiblity, or non-starters to this idea

[pmcpinto]

Will this product type include all the Core features by default?
If an extension uses this product module, will a new type be introduced in the product type selector?

[joshuatf]

Will this product type include all the Core features by default?

Yes, this would include all core features.

If an extension uses this product module, will a new type be introduced in the product type selector?

The main purpose of this concept is to remove "type" selection for users and reduce the complexity of the UI. In some cases, extensions may want to just introduce a toggle or inherently turn on/off a module based on interaction with a field (e.g., variations automatically turns on/off depending on whether variations exist).

However, the option always exists to include a new "template" in the dropdown, which can be what we think of as "types" or can be a cobination of preset data to help merchants more quickly create a product.

[barryhughes]

Small thing: might a classname be preferable to a slug? Depending on how get_module is implemented, this could be used to guarantee the return type (that is, if we ask for foo::class we can expect to get back either an instance of foo or else false).

- $product->get_module( 'subscriptions' );
+ $product->get_module( Subscriptions_Module::class );

[joshuatf]

I think this has its advantages. The one downside to this is the need to check classes (at least on third party integrated modules) before retrieving a module, but this is a check that needs to occur at some point regardless.

[barryhughes]

If a module is not activated, does that prohibit interaction?

For example, if a hypothetical downloads module is initially activated, and downloadable assets are added/registered, and then for some reason the module is deactivated ... can we still access the module and its methods, or would that be prohibited?

[joshuatf]

Good question. In earlier concepts I explored both the idea that (some) methods could be restricted depending on whether or not the module was activated and also that all methods could passthrough from the product to the traits.

In the latest POC, I've strayed away from that idea because I think it leads to an inconsistent interface. So in short, those methods should always be available and not dependent on module activation.

[joshuatf]

@daigo75 I see you left a thumbs down on this discussion and I understand you might have some concerns over backwards compatibility.

The current concept attempts to remove any issues with backwards compatibility by creating a separate type (making this new modular type opt-in and not affecting other types) but still allowing classic get_type checks for plugins that may be relying on that check. This has worked in a (admittedly narrow) set of use cases with extensions, but I'd love to hear your feedback on where you think this might cause issues.

I want to stress that this is purely exploratory at the moment and not something we would implement if it would cause issues in backwards compatibility. If you have use cases you're concerned about, it would be great to share those so we can discuss and make those cases are covered.

[daigo75]

I'm quite busy these days, and I regret that I don't have much time for a long conversation on the subject, so I will try to be brief. One use case I can see causing trouble is the reliance of the existing product classes. I've personally seen quite a few of plugins and customisations that rely on specific product classes, with their own properties and interfaces, and extend them.

Something as simple as this:

class My_Product extends WC_Product_Simple {
  // Some stuff
}

Could stop working, and it would have to be rewritten to be compatible with the "old" structure and the new one. It could be a serious breaking change, like the one that they made in PHP 8.2 when they removed dynamic properties. In one PHP version, you can use them. In the next one (a minor update, a x.2) you can't. I voted against that change too, and twice as much because the PHP developer was on his way out. In that case, all the opposition was met with "but you shouldn't have used dynamic properties". The way I see it, it was perfectly fine to do use dynamic properties, at least until a single individual decided it wasn't, therefore "you shouldn't do that" wasn't a valid answer for me. They provided a "workaround" to enable dynamic properties on specific classes, but they didn't take into account that one might not be able to modify the code of these classes. Due to that, the workaround is practically useless, unless one has full control over the whole codebase (which rarely happens, if one works with 3rd party libraries or code, such as WooCommerce).

In short, I would like to avoid the same mess. To guarantee backward compatibility with the code that relies on the current architecture, the existing classes will have be preserved, with their existing interfaces and behaviour. If that's not possible, then the knock on effect will be very serious, and I don't see the benefit in the new approach.

[joshuatf]

Thanks for your thoughts, Diego!!

Something as simple as this: class My_Product extends WC_Product_Simple {

I think your concern about this type of break is valid, but this would still work given these classes and types still exist. While my earlier discussions and POCs did favor deprecating product types, the concept in this post creates a new type that is more modular. Plugins can continue using types and existing classes as is. Programmatically, very little has to change for extensions that are creating their own types unless they want to opt into the modular product experience.

My larger backwards compatibility concerns are around extensions doing something funky with extending existing type functionality in a manner that's less than ideal:

function add_something_to_product( $product ) {
  if ( $product->get_type() === 'simple' ) { ... } // This would work fine with modular products

  if ( is_a( $product, 'WC_Product_Simple' ) ) { ... } // This would not work since the class would be `WC_Product_Modular`
}

I have yet to see this exact case in the wild, but I don't doubt that it might exist. And if it does exist, I'd like to see what impact that has as it may be something that does not pose major issues or it may be a dealbreaker. It may also be acceptable given we're working with a new type altogether.

To guarantee backward compatibility with the code that relies on the current architecture, the existing classes will have be preserved, with their existing interfaces and behaviour.
✅ ✅ ✅

and I don't see the benefit in the new approach.

Not sure if this was a continuation of your previous thought and contigent on breaking interface, but just noting that a bigger part of this change is less about developer benefits (though I do think it pushes towards a more consistent interface long-term) and more about how this relates to merchant interface.

Product types offload a good amount of responsibility to merchants to figure out how to correctly configure products. What has become abundantly clear in user testing is that the current UI is less than ideal and a major pain point in creating a better UI is type selection.

And again, none of that is to say that this is the best solution; it's just one idea that moves towards solving that issue. The more cases we can discuss around it, the better we can vet out whether or not it's worth pursuing. 🙂

[daigo75]

I can tell you that is_a() and instanceof are in use. I used the latter them myself, back when there was no get_type() function, and the code is running smoothly to this day.

In addition to that, functions could expect a WC_Product_Simple argument, or a WC_Product_Variable, or something like that.

If you plan to have a WC_Product_Simple, and other classes, that extend WC_Product_Modular, that might work. If not, that's going to be an issue.

Furthermore, building product types that may or may not have specific features will make it tricky to support them. My day to day work revolves around handling custom prices for products , and that can only be done by knowing what a product is and what it does. Simple products are handled in one way, variable in a slightly different way, and so on. Small digression: bundles, subscriptions, composites, etc should be handled by the respective authors, who know how these products work, even if that hasn't yet happened with some plugins. End of digression.

That would be about it, I think.

[joshuatf]

If you plan to have a WC_Product_Simple, and other classes, that extend WC_Product_Modular, that might work. If not, that's going to be an issue.

There will still be a WC_Product_Simple class. What other classes are you expecting to extend WC_Product_Modular or did I misunderstand you here?

Furthermore, building product types that may or may not have specific features will make it tricky to support them.

We already have this to a large extent with the product factory which is why extensions check the type and/or class of a product to determine what features are available. I don't believe that's inherently different than checking if a module is active on a class; it's just a different way of checking. You just as easily be able to check by running $product->is_module_active( Variable::class ).

While there are many cases of product types that probably should not have been made types and can't easily retroactively convert to non-types, there are even more cases of plugins extended existing types with features that are not compatible with one another.
The advantage to the modular approach is being able to declare 2 "modules" or "traits" compatible or incompatible, solving a lot of these ongoing compatibility issues between third party plugins.

But I hear you on the legacy code and nuisance that adding another type check would represent. I'm not yet convinced that the modular type is the right approach, but it is very clear that we need a better way of handling compatibility between product features (not just types).

[EdithAllison]

@joshuatf The use case that worries you is used here: https://brickslabs.com/create-a-customizable-ajax-add-to-cart-button-in-bricks-builder/

[joshuatf]

Thanks for the example, Edith!

In this particular example (which I think highlights the majority of cases), it seems like the functionality added when itchy_is_simple_product() is true would only apply to simple products. So this would continue to work with simple products, but itchy_is_simple_product would have to be extended if products of the new modular type want to be included.

I think this is good and expected behavior in most ways as it prevents breaking changes, but I can also see the case where it's a nuisance for developers to need to include one more product type check. This is something we'll need to weigh out and see if the perceived UI benefits are worth this added cost.