New Transformer: AutoExtensions

[schlessera]

• Automatically add scripts for used extensions
• Strip unused extension scripts

[saitho]

I've started working on this. I already did that for my CMS and I am trying to bring that into the amp-toolbox-php.

I noticed there are AMP extensions that define additional elements (e.g. amp-bind defining both amp-bind and amp-bind-macro elements). My current approach is to fetch the latest AMP release and extract a map "element => extension" for those elements that are not the same as the extension name.

Resulting in a list like that:

const AMP_EXTENSIONS_MAP = [
        // amp-ad
        "amp-embed" => "amp-ad",

        // amp-bind
        "amp-state" => "amp-bind",
        "amp-bind-macro" => "amp-bind",

        // amp-embedly-card
        "amp-embedly-key" => "amp-embedly-card",

        // amp-google-assistant-assistjs
        "amp-google-assistant-assistjs-config" => "amp-google-assistant-assistjs",
        "amp-google-assistant-voice-button" => "amp-google-assistant-assistjs",
        "amp-google-assistant-voice-bar" => "amp-google-assistant-assistjs",
        "amp-google-assistant-inline-suggestion-bar" => "amp-google-assistant-assistjs"

// ...
];

The generated PHP file would be part of the release (i.e. the repository) and can be updated via a NPM command.
The constants inside that file are then used for mapping the detected AMP element to the extension name in the AutoExtensionsTransformer.

Would that also be a sufficient solution here? If so, should I generate a PHP file (maybe even use that to generate the Extension interface file?) with the constants or a different file format (JSON/YAML)?

[westonruter]

You're on the right track, but I think most of what you're suggesting is already mostly complete in #100. The extension dependencies are encoded in the validator spec, and once that spec is available in PHP form it can be used to do the auto-extension insertion.

So you can see in AmpState.php that it defines the amp-state tag:

amp-toolbox-php/src/Validator/Spec/Tag/AmpState.php

Lines 20 to 25 in c8042c0

	/**
	* ID of the tag.
	*
	* @var string
	*/
	const ID = 'amp-state';

And it specifies that it requires the amp-bind extension:

amp-toolbox-php/src/Validator/Spec/Tag/AmpState.php

Lines 68 to 70 in c8042c0

	SpecRule::REQUIRES_EXTENSION => [
	Extension::BIND,
	],

Once that is merged, creating the AutoExtensionsTransformer will be straightforward as all the data will be available to us.

[saitho]

That looks promising, thanks. I'll take a look at it and wait for the merge then. :)

[schlessera]

Hey @saitho,

I already have a PR in the works for the auto-extensions as well. You can currently see it in the https://github.com/ampproject/amp-toolbox-php/tree/add/32-auto-extensions branch right now. It was just blocked because the validator spec is indeed needed to make this work across all edge cases.

So, before you put any work into this, please wait for me to push everything into its final state. I'll ping you on the new PR for the auto-extensions so we can exchange approaches and discuss implementation details.

[schlessera]

Sorry, the link above was not very helpful, here is a better one that shows the current changes for this new auto-extensions transformer: add/amp-validator-spec...add/32-auto-extensions

[saitho]

Sure, thanks!

I've already implemented my approach on my end outside of the amp-toolbox. Will try your branch when it's ready. :)

[schlessera]

I've turned the branch into a draft PR for a better overview: #210