Skip to content

Commit 65bcb02

Browse files
magento-engcom-teammagento-engcom-team
committed
ENGCOM-7154: Adjust mechanism that moves all scripts to the end of the page #27270
- Merge Pull Request #27270 from krzksz/magento2:defer-js - Merged commits: 1. 31eeaa7 2. 4fa5447 3. 08f2bd4 4. 2c5bc50 5. 27805df 6. 693507f 7. 31b0456 8. b54ce80 9. 036f99f 10. 9687370 11. e38b1a1 12. 1b780be 13. b7ff624 14. ffd392c 15. d963abc
2 parents b7c9e07 + d963abc commit 65bcb02

File tree

3 files changed

+106
-66
lines changed

3 files changed

+106
-66
lines changed

‎app/code/Magento/Theme/Controller/Result/JsFooterPlugin.php‎

Lines changed: 71 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88
namespace Magento\Theme\Controller\Result;
99

1010
use Magento\Framework\App\Config\ScopeConfigInterface;
11+
use Magento\Framework\App\Response\HttpInterface as HttpResponseInterface;
12+
use Magento\Framework\App\ResponseInterface;
13+
use Magento\Framework\View\Result\Layout;
1114
use Magento\Store\Model\ScopeInterface;
12-
use Magento\Framework\App\Response\Http;
1315

1416
/**
15-
* Plugin for putting all js to footer.
17+
* Plugin for putting all JavaScript tags to the end of body.
1618
*/
1719
class JsFooterPlugin
1820
{
@@ -32,34 +34,77 @@ public function __construct(ScopeConfigInterface $scopeConfig)
3234
}
3335

3436
/**
35-
* Put all javascript to footer before sending the response.
37+
* Moves all JavaScript tags to the end of body if this feature is enabled.
3638
*
37-
* @param Http $subject
38-
* @return void
39+
* @param Layout $subject
40+
* @param Layout $result
41+
* @param HttpResponseInterface|ResponseInterface $httpResponse
42+
* @return Layout (That should be void, actually)
43+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
3944
*/
40-
public function beforeSendResponse(Http $subject)
45+
public function afterRenderResult(Layout $subject, Layout $result, ResponseInterface $httpResponse)
4146
{
42-
$content = $subject->getContent();
43-
$script = [];
44-
if (is_string($content) && strpos($content, '</body') !== false) {
45-
if ($this->scopeConfig->isSetFlag(
46-
self::XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
47-
ScopeInterface::SCOPE_STORE
48-
)
49-
) {
50-
$pattern = '#<script[^>]*+(?<!text/x-magento-template.)>.*?</script>#is';
51-
$content = preg_replace_callback(
52-
$pattern,
53-
function ($matchPart) use (&$script) {
54-
$script[] = $matchPart[0];
55-
return '';
56-
},
57-
$content
58-
);
59-
$subject->setContent(
60-
str_replace('</body', implode("\n", $script) . "\n</body", $content)
61-
);
47+
if (!$this->isDeferEnabled()) {
48+
return $result;
49+
}
50+
51+
$content = (string)$httpResponse->getContent();
52+
$bodyEndTag = '</body';
53+
$bodyEndTagFound = strrpos($content, $bodyEndTag) !== false;
54+
55+
if ($bodyEndTagFound) {
56+
$scripts = $this->extractScriptTags($content);
57+
if ($scripts) {
58+
$newBodyEndTagPosition = strrpos($content, $bodyEndTag);
59+
$content = substr_replace($content, $scripts . "\n", $newBodyEndTagPosition, 0);
60+
$httpResponse->setContent($content);
61+
}
62+
}
63+
64+
return $result;
65+
}
66+
67+
/**
68+
* Extracts and returns script tags found in given content.
69+
*
70+
* @param string $content
71+
*/
72+
private function extractScriptTags(&$content): string
73+
{
74+
$scripts = '';
75+
$scriptOpen = '<script';
76+
$scriptClose = '</script>';
77+
$scriptOpenPos = strpos($content, $scriptOpen);
78+
79+
while ($scriptOpenPos !== false) {
80+
$scriptClosePos = strpos($content, $scriptClose, $scriptOpenPos);
81+
$script = substr($content, $scriptOpenPos, $scriptClosePos - $scriptOpenPos + strlen($scriptClose));
82+
$isXMagentoTemplate = strpos($script, 'text/x-magento-template') !== false;
83+
84+
if ($isXMagentoTemplate) {
85+
$scriptOpenPos = strpos($content, $scriptOpen, $scriptClosePos);
86+
continue;
6287
}
88+
89+
$scripts .= "\n" . $script;
90+
$content = str_replace($script, '', $content);
91+
// Script cut out, continue search from its position.
92+
$scriptOpenPos = strpos($content, $scriptOpen, $scriptOpenPos);
6393
}
94+
95+
return $scripts;
96+
}
97+
98+
/**
99+
* Returns information whether moving JS to footer is enabled
100+
*
101+
* @return bool
102+
*/
103+
private function isDeferEnabled(): bool
104+
{
105+
return $this->scopeConfig->isSetFlag(
106+
self::XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
107+
ScopeInterface::SCOPE_STORE
108+
);
64109
}
65110
}

‎app/code/Magento/Theme/Test/Unit/Controller/Result/JsFooterPluginTest.php‎

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Framework\App\Config\ScopeConfigInterface;
1111
use Magento\Framework\App\Response\Http;
1212
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
13+
use Magento\Framework\View\Result\Layout;
1314
use Magento\Store\Model\ScopeInterface;
1415
use Magento\Theme\Controller\Result\JsFooterPlugin;
1516
use PHPUnit\Framework\MockObject\MockObject;
@@ -22,21 +23,18 @@ class JsFooterPluginTest extends TestCase
2223
{
2324
const STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM = 'dev/js/move_script_to_bottom';
2425

25-
/**
26-
* @var JsFooterPlugin
27-
*/
26+
/** @var JsFooterPlugin */
2827
private $plugin;
2928

30-
/**
31-
* @var ScopeConfigInterface|MockObject
32-
*/
29+
/** @var ScopeConfigInterface|MockObject */
3330
private $scopeConfigMock;
3431

35-
/**
36-
* @var Http|MockObject
37-
*/
32+
/** @var Http|MockObject */
3833
private $httpMock;
3934

35+
/** @var Layout|MockObject */
36+
private $layoutMock;
37+
4038
/**
4139
* @inheritdoc
4240
*/
@@ -48,6 +46,7 @@ protected function setUp(): void
4846
->getMockForAbstractClass();
4947

5048
$this->httpMock = $this->createMock(Http::class);
49+
$this->layoutMock = $this->createMock(Layout::class);
5150

5251
$objectManager = new ObjectManagerHelper($this);
5352
$this->plugin = $objectManager->getObject(
@@ -59,11 +58,11 @@ protected function setUp(): void
5958
}
6059

6160
/**
62-
* Data Provider for testBeforeSendResponse()
61+
* Data Provider for testAfterRenderResult()
6362
*
6463
* @return array
6564
*/
66-
public function sendResponseDataProvider(): array
65+
public function renderResultDataProvider(): array
6766
{
6867
return [
6968
'content_with_script_tag' => [
@@ -74,9 +73,9 @@ public function sendResponseDataProvider(): array
7473
"flag" => true,
7574
"result" => "<body><h1>Test Title</h1>" .
7675
"<script type=\"text/x-magento-template\">test</script>" .
77-
"<p>Test Content</p>" .
78-
"<script type=\"text/x-magento-init\">test</script>" .
79-
"\n</body>"
76+
"<p>Test Content</p>\n" .
77+
"<script type=\"text/x-magento-init\">test</script>\n" .
78+
"</body>"
8079
],
8180
'content_with_config_disable' => [
8281
"content" => "<body><p>Test Content</p></body>",
@@ -98,67 +97,61 @@ public function sendResponseDataProvider(): array
9897
* @param bool $isSetFlag
9998
* @param string $result
10099
* @return void
101-
* @dataProvider sendResponseDataProvider
100+
* @dataProvider renderResultDataProvider
102101
*/
103-
public function testBeforeSendResponse($content, $isSetFlag, $result): void
102+
public function testAfterRenderResult($content, $isSetFlag, $result): void
104103
{
105-
$this->httpMock->expects($this->once())
106-
->method('getContent')
104+
// Given (context)
105+
$this->httpMock->method('getContent')
107106
->willReturn($content);
108107

109-
$this->scopeConfigMock->expects($this->once())
110-
->method('isSetFlag')
111-
->with(
112-
self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
113-
ScopeInterface::SCOPE_STORE
114-
)
108+
$this->scopeConfigMock->method('isSetFlag')
109+
->with(self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM, ScopeInterface::SCOPE_STORE)
115110
->willReturn($isSetFlag);
116111

112+
// Expects
117113
$this->httpMock->expects($this->any())
118114
->method('setContent')
119115
->with($result);
120116

121-
$this->plugin->beforeSendResponse($this->httpMock);
117+
// When
118+
$this->plugin->afterRenderResult($this->layoutMock, $this->layoutMock, $this->httpMock);
122119
}
123120

124121
/**
125-
* Data Provider for testBeforeSendResponseIfGetContentIsNotAString()
122+
* Data Provider for testAfterRenderResultIfGetContentIsNotAString()
126123
*
127124
* @return array
128125
*/
129126
public function ifGetContentIsNotAStringDataProvider(): array
130127
{
131128
return [
132-
'empty_array' => [
133-
'content' => []
134-
],
135129
'null' => [
136130
'content' => null
137131
]
138132
];
139133
}
140134

141135
/**
142-
* Test BeforeSendResponse if content is not a string
136+
* Test AfterRenderResult if content is not a string
143137
*
144138
* @param string $content
145139
* @return void
146140
* @dataProvider ifGetContentIsNotAStringDataProvider
147141
*/
148-
public function testBeforeSendResponseIfGetContentIsNotAString($content): void
142+
public function testAfterRenderResultIfGetContentIsNotAString($content): void
149143
{
144+
$this->scopeConfigMock->method('isSetFlag')
145+
->with(self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM, ScopeInterface::SCOPE_STORE)
146+
->willReturn(true);
147+
150148
$this->httpMock->expects($this->once())
151149
->method('getContent')
152150
->willReturn($content);
153151

154-
$this->scopeConfigMock->expects($this->never())
155-
->method('isSetFlag')
156-
->with(
157-
self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
158-
ScopeInterface::SCOPE_STORE
159-
)
160-
->willReturn(false);
152+
$this->httpMock->expects($this->never())
153+
->method('setContent');
161154

162-
$this->plugin->beforeSendResponse($this->httpMock);
155+
$this->plugin->afterRenderResult($this->layoutMock, $this->layoutMock, $this->httpMock);
163156
}
164157
}

‎app/code/Magento/Theme/etc/frontend/di.xml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
<plugin name="result-messages" type="Magento\Theme\Controller\Result\MessagePlugin"/>
2828
</type>
2929
<type name="Magento\Framework\App\Response\Http">
30-
<plugin name="result-js-footer" type="Magento\Theme\Controller\Result\JsFooterPlugin"/>
3130
<plugin name="asyncCssLoad" type="Magento\Theme\Controller\Result\AsyncCssPlugin"/>
3231
</type>
32+
<type name="Magento\Framework\View\Result\Layout">
33+
<plugin name="deferJsToFooter" type="Magento\Theme\Controller\Result\JsFooterPlugin" sortOrder="-10"/>
34+
</type>
3335
<type name="Magento\Theme\Block\Html\Header\CriticalCss">
3436
<arguments>
3537
<argument name="filePath" xsi:type="string">css/critical.css</argument>

0 commit comments

Comments
 (0)