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

Add dependency resolution and ordering for transformers #219

Open
schlessera opened this issue Jun 1, 2021 · 5 comments
Open

Add dependency resolution and ordering for transformers #219

schlessera opened this issue Jun 1, 2021 · 5 comments

Comments

@schlessera
Copy link
Collaborator

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.

@06romix
Copy link
Contributor

06romix commented Jun 1, 2021

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

@schlessera
Copy link
Collaborator Author

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

@06romix
Copy link
Contributor

06romix commented Jun 3, 2021

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.

@westonruter
Copy link
Member

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

@schlessera
Copy link
Collaborator Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants