amp-toolbox-php icon indicating copy to clipboard operation
amp-toolbox-php copied to clipboard

Add dependency resolution and ordering for transformers

Open schlessera opened this issue 3 years ago • 5 comments

Based on a discussion in Slack with @06romix:

What I was thinking of was to have two interfaces with corresponding methods:

interface Requires {
    /**
     * Return the collection of dependencies that this transformer requires.
     *
     * @return string[]
     */
    public static function requires();
}
interface Provides {
    /**
     * Return the collection of dependencies that this transformer provides.
     *
     * @return string[]
     */
    public static function provides();
}

The interfaces can be checked with instanceof without even instantiating the classes. The strings that are returned can be thought of as "tags". So, one or more transformers could require 'ssr', and the ServersideRendering transformer would provide 'ssr'. If someone replaces that transformer with a custom one, they can still have it provide 'ssr' to fulfil the dependency chain. The ordering can then happen based on these tags, provided that it is resolvable. If it is not, there is obviously already a problem that needs to be solved.

schlessera avatar Jun 01 '21 18:06 schlessera

If ServerSideRendering required PreloadHeroImages would we be able to skip PreloadHeroImages?

06romix avatar Jun 01 '21 19:06 06romix

If ServerSideRendering required PreloadHeroImages, and you'd skip PreloadHeroImages without skipping ServerSideRendering, the engine would throw an error.

schlessera avatar Jun 01 '21 19:06 schlessera

Ok, good idea. One thing that worries me, its tags. Would it be comfortable using 'ssr', 'phi', 'oab', 'ab', 'arc' or could we use 'server_side_rendering' and 'preload_hero_images' as tags.

06romix avatar Jun 03 '21 18:06 06romix

Why use tags when you could just use the transformer class names?

westonruter avatar Jun 05 '21 17:06 westonruter

@westonruter: Using the transformer class would directly couple the dependency resolution to the implementations, and would make it impossible, for example, to extend/replace a transformer that another transformer depends on.

So, if someone provides a custom implementation of SSR, the transformers depending on SSR should still be able to work together with that custom implementation.

schlessera avatar Jun 06 '21 10:06 schlessera