Add a new Link component #22273
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <fabien@symfony.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Bridge\Twig\Extension; | ||
|
|
||
| use Fig\Link\GenericLinkProvider; | ||
| use Fig\Link\Link; | ||
| use Symfony\Component\HttpFoundation\RequestStack; | ||
|
|
||
| /** | ||
| * Twig extension for the Symfony WebLink component. | ||
| * | ||
| * @author Kévin Dunglas <dunglas@gmail.com> | ||
| */ | ||
| class WebLinkExtension extends \Twig_Extension | ||
| { | ||
| private $requestStack; | ||
|
|
||
| public function __construct(RequestStack $requestStack) | ||
| { | ||
| $this->requestStack = $requestStack; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function getFunctions() | ||
| { | ||
| return array( | ||
| new \Twig_SimpleFunction('link', array($this, 'link')), | ||
| new \Twig_SimpleFunction('preload', array($this, 'preload')), | ||
| new \Twig_SimpleFunction('dns_prefetch', array($this, 'dnsPrefetch')), | ||
| new \Twig_SimpleFunction('preconnect', array($this, 'preconnect')), | ||
| new \Twig_SimpleFunction('prefetch', array($this, 'prefetch')), | ||
| new \Twig_SimpleFunction('prerender', array($this, 'prerender')), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Adds a "Link" HTTP header. | ||
| * | ||
| * @param string $uri The relation URI | ||
| * @param string $rel The relation type (e.g. "preload", "prefetch", "prerender" or "dns-prefetch") | ||
| * @param array $attributes The attributes of this link (e.g. "array('as' => true)", "array('pr' => 0.5)") | ||
| * | ||
| * @return string The relation URI | ||
| */ | ||
| public function link($uri, $rel, array $attributes = array()) | ||
| { | ||
| if (!$request = $this->requestStack->getMasterRequest()) { | ||
| return $uri; | ||
| } | ||
|
|
||
| $link = new Link($rel, $uri); | ||
| foreach ($attributes as $key => $value) { | ||
| $link = $link->withAttribute($key, $value); | ||
| } | ||
|
|
||
| $linkProvider = $request->attributes->get('_links', new GenericLinkProvider()); | ||
|
There was a problem hiding this comment. Is this ok or should we always populate this key? The current implementation is more memory efficient. There was a problem hiding this comment. Actually, I really think the ideal case would be for But I understand that's not probably not possible considering the mutable nature of our There was a problem hiding this comment.
I agree but as you pointed out, it's not possible.
Why? |
||
| $request->attributes->set('_links', $linkProvider->withLink($link)); | ||
|
|
||
| return $uri; | ||
| } | ||
|
|
||
| /** | ||
| * Preloads a resource. | ||
| * | ||
| * @param string $uri A public path | ||
| * @param array $attributes The attributes of this link (e.g. "array('as' => true)", "array('crossorigin' => 'use-credentials')") | ||
| * | ||
| * @return string The path of the asset | ||
| */ | ||
| public function preload($uri, array $attributes = array()) | ||
| { | ||
| return $this->link($uri, 'preload', $attributes); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves a resource origin as early as possible. | ||
| * | ||
| * @param string $uri A public path | ||
| * @param array $attributes The attributes of this link (e.g. "array('as' => true)", "array('pr' => 0.5)") | ||
| * | ||
| * @return string The path of the asset | ||
| */ | ||
| public function dnsPrefetch($uri, array $attributes = array()) | ||
| { | ||
| return $this->link($uri, 'dns-prefetch', $attributes); | ||
| } | ||
|
|
||
| /** | ||
| * Initiates a early connection to a resource (DNS resolution, TCP handshake, TLS negotiation). | ||
| * | ||
| * @param string $uri A public path | ||
| * @param array $attributes The attributes of this link (e.g. "array('as' => true)", "array('pr' => 0.5)") | ||
| * | ||
| * @return string The path of the asset | ||
| */ | ||
| public function preconnect($uri, array $attributes = array()) | ||
| { | ||
| return $this->link($uri, 'preconnect', $attributes); | ||
| } | ||
|
|
||
| /** | ||
| * Indicates to the client that it should prefetch this resource. | ||
| * | ||
| * @param string $uri A public path | ||
| * @param array $attributes The attributes of this link (e.g. "array('as' => true)", "array('pr' => 0.5)") | ||
| * | ||
| * @return string The path of the asset | ||
| */ | ||
| public function prefetch($uri, array $attributes = array()) | ||
| { | ||
| return $this->link($uri, 'prefetch', $attributes); | ||
| } | ||
|
|
||
| /** | ||
| * Indicates to the client that it should prerender this resource . | ||
| * | ||
| * @param string $uri A public path | ||
| * @param array $attributes The attributes of this link (e.g. "array('as' => true)", "array('pr' => 0.5)") | ||
| * | ||
| * @return string The path of the asset | ||
| */ | ||
| public function prerender($uri, array $attributes = array()) | ||
| { | ||
| return $this->link($uri, 'prerender', $attributes); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <fabien@symfony.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Bridge\Twig\Tests\Extension; | ||
|
|
||
| use Fig\Link\Link; | ||
| use PHPUnit\Framework\TestCase; | ||
| use Symfony\Bridge\Twig\Extension\WebLinkExtension; | ||
| use Symfony\Component\HttpFoundation\Request; | ||
| use Symfony\Component\HttpFoundation\RequestStack; | ||
|
|
||
| /** | ||
| * @author Kévin Dunglas <dunglas@gmail.com> | ||
| */ | ||
| class WebLinkExtensionTest extends TestCase | ||
| { | ||
| /** | ||
| * @var Request | ||
| */ | ||
| private $request; | ||
|
|
||
| /** | ||
| * @var WebLinkExtension | ||
| */ | ||
| private $extension; | ||
|
|
||
| protected function setUp() | ||
| { | ||
| $this->request = new Request(); | ||
|
|
||
| $requestStack = new RequestStack(); | ||
| $requestStack->push($this->request); | ||
|
|
||
| $this->extension = new WebLinkExtension($requestStack); | ||
| } | ||
|
|
||
| public function testLink() | ||
| { | ||
| $this->assertEquals('/foo.css', $this->extension->link('/foo.css', 'preload', array('as' => 'style', 'nopush' => true))); | ||
|
|
||
| $link = (new Link('preload', '/foo.css'))->withAttribute('as', 'style')->withAttribute('nopush', true); | ||
| $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks())); | ||
| } | ||
|
|
||
| public function testPreload() | ||
| { | ||
| $this->assertEquals('/foo.css', $this->extension->preload('/foo.css', array('as' => 'style', 'crossorigin' => true))); | ||
|
|
||
| $link = (new Link('preload', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true); | ||
| $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks())); | ||
| } | ||
|
|
||
| public function testDnsPrefetch() | ||
| { | ||
| $this->assertEquals('/foo.css', $this->extension->dnsPrefetch('/foo.css', array('as' => 'style', 'crossorigin' => true))); | ||
|
|
||
| $link = (new Link('dns-prefetch', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true); | ||
| $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks())); | ||
| } | ||
|
|
||
| public function testPreconnect() | ||
| { | ||
| $this->assertEquals('/foo.css', $this->extension->preconnect('/foo.css', array('as' => 'style', 'crossorigin' => true))); | ||
|
|
||
| $link = (new Link('preconnect', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true); | ||
| $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks())); | ||
| } | ||
|
|
||
| public function testPrefetch() | ||
| { | ||
| $this->assertEquals('/foo.css', $this->extension->prefetch('/foo.css', array('as' => 'style', 'crossorigin' => true))); | ||
|
|
||
| $link = (new Link('prefetch', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true); | ||
| $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks())); | ||
| } | ||
|
|
||
| public function testPrerender() | ||
| { | ||
| $this->assertEquals('/foo.css', $this->extension->prerender('/foo.css', array('as' => 'style', 'crossorigin' => true))); | ||
|
|
||
| $link = (new Link('prerender', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true); | ||
| $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks())); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this dep is in require-dev in all components, but in require here, looks suspicious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this package in the Twig extension and the FrameworkBundle (even at runtime). But the those packages have only a soft dependency to WebLink. In the context of the full stack, I think it makes sense to add it to have something working out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not
required by any of the packages inreplace, so it should indeed remain inrequire-dev.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is required if you use WebLink && (Twig Bridge || Framework Bundle). We cannot express such conditions with Composer, but the full stack framework install WebLink + Twig Bridge + Framework Bundle so it needs to have this package as dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still looks inconsistent to me. symfony/symfony should not behave differently than using standalone components...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fig/link-utiladded back as a dependency of the WebLink component. It fixes the inconsistency, makes the component easier to use for end users and... it's the only one implementation of PSR-13 for now.