Add resource hints transformers #179
Conversation
schlessera
changed the title
| private function isAmpRuntimeScriptNeeded(Document $document) | ||
| { | ||
| // TODO: What condition should be used here... same as isAmpRuntimeCssNeeded()? | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Yes, I'm not sure yet what the best practices are here.
| if ($this->isAmpRuntimeScriptNeeded($document)) { | ||
| $document->links->addPreload($this->getAmpRuntimeScriptHost(), RequestDestination::SCRIPT); | ||
| } |
There was a problem hiding this comment.
This is currently being handled by the RewriteAmpUrls transformer. Should the logic be moved here? At the very least it seems RewriteAmpUrls should make use of the LinkManager in its createPreload method.
There was a problem hiding this comment.
I made RewriteAmpUrls reuse the link manager. I also extended the link manager so that it ensure no duplicate links are being produced. This currently uses both href and rel for the uniqueness key.
There was a problem hiding this comment.
While this isn't important right now, it is possible for the rel to be list of tokens and not just a single value. That could cause problems here.
schlessera
force-pushed
the
add/31-browser-hints-transformer
branch
from
a2f6e51
to
6c91427
Compare
| if ($this->isAmpRuntimeScriptNeeded($document)) { | ||
| $document->links->addPreload($this->getAmpRuntimeScriptHost(), RequestDestination::SCRIPT); | ||
| } |
There was a problem hiding this comment.
While this isn't important right now, it is possible for the rel to be list of tokens and not just a single value. That could cause problems here.
| if (! $link instanceof Element) { | ||
| throw FailedToCreateLink::forLink($link); | ||
| } |
There was a problem hiding this comment.
How would this condition ever happen?
There was a problem hiding this comment.
If you do something fishy with the document, you might end up with a DOMElement that is not an Element.
Also, PhpStan is happier with this restriction.
Co-authored-by: Weston Ruter <westonruter@google.com>
schlessera
force-pushed
the
add/31-browser-hints-transformer
branch
from
1021344
to
5831c16
Compare
This PR adds two transformers to the Optimizer:
AmpRuntimePreloadsThis adds preloads for the AMP runtime scripts (WIP) and styles, if they are needed.
GoogleFontsPreconnectThis adds a preconnect for the Google Fonts domain if Google Fonts are being used, and adds a DNS prefetch as a fallback.
It also adds an abstraction for managing resource hints via the
Dom\Document:TODOS:
preconnectwithout the best-practice-adviseddns-prefetch?prefetch,prerender, 'dns-prefetch` on its own...?