Skip to content

Commit

Permalink
Prevent double-registering of asset bundles and JS files in the CP
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonkelly committed Jun 1, 2018
1 parent 9f25a19 commit 6b446e8
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 8 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG-v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Added
- Added `craft\helpers\Stringy::getLangSpecificCharsArray()`.
- Added `craft\web\View::setRegisteredAssetBundles()`.
- Added `craft\web\View::setRegisteredJsFiles()`.

### Changed
- Generated site URLs now always include full host info, even if the base site URL is root/protocol-relative. ([#2919](https://github.com/craftcms/cms/issues/2919))
Expand All @@ -24,6 +26,7 @@
- Fixed a bug where `craft\helpers\StringHelper::toAscii()` and the `Craft.asciiString()` JS method weren’t using language-specific character replacements, or any custom replacements defined by the `customAsciiCharMappings` config setting.
- Fixed a bug where the number `0` would not save in a Plain Text field.
- Fixed a bug where Craft could pick the wrong current site if the primary site had a root-relative or protocol-relative URL, and another site didn’t, but was otherwise an equal match.
- Fixed a bug where Control Panel Ajax requests could cause some asset bundles and JavaScript files to be double-registered in the browser.

## 3.0.9 - 2018-05-22

Expand Down
18 changes: 15 additions & 3 deletions src/config/app.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@
'users' => [
'class' => craft\services\Users::class,
],
'view' => [
'class' => craft\web\View::class,
],
'volumes' => [
'class' => craft\services\Volumes::class,
],
Expand Down Expand Up @@ -263,5 +260,20 @@
]
]);
},

'view' => function() {
$config = [
'class' => craft\web\View::class,
];

$request = Craft::$app->getRequest();
if ($request->getIsCpRequest()) {
$headers = $request->getHeaders();
$config['registeredAssetBundles'] = explode(',', $headers->get('X-Registered-Asset-Bundles', ''));
$config['registeredJsFiles'] = explode(',', $headers->get('X-Registered-Js-Files', ''));
}

return Craft::createObject($config);
},
],
];
84 changes: 84 additions & 0 deletions src/web/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
* @property-read Environment $twig the Twig environment
* @property-read string $bodyHtml the content to be inserted at the end of the body section
* @property-read string $headHtml the content to be inserted in the head section
* @property-write string[] $registeredAssetBundles the asset bundle names that should be marked as already registered
* @property-write string[] $registeredJsFiles the JS files that should be marked as already registered
* @author Pixel & Tonic, Inc. <support@pixelandtonic.com>
* @since 3.0
*/
Expand Down Expand Up @@ -196,6 +198,20 @@ class View extends \yii\web\View
*/
private $_isRenderingPageTemplate = false;

/**
* @var string[]
* @see registerAssetFiles()
* @see setRegisteredAssetBundles()
*/
private $_registeredAssetBundles = [];

/**
* @var string[]
* @see registerJsFile()
* @see setRegisteredJsfiles()
*/
private $_registeredJsFiles = [];

// Public Methods
// =========================================================================

Expand Down Expand Up @@ -791,6 +807,19 @@ public function clearJsBuffer(bool $scriptTag = true)
return $js;
}

/**
* @inheritdoc
*/
public function registerJsFile($url, $options = [], $key = null)
{
$key = $key ?: $url;
if (isset($this->_registeredJsFiles[$key])) {
return;
}
$this->_registeredJsFiles[$key] = true;
parent::registerJsFile($url, $options, $key);
}

/**
* Registers a generic `<script>` code block.
*
Expand Down Expand Up @@ -1206,6 +1235,26 @@ public function invokeHook(string $hook, array &$context): string
return $return;
}

/**
* Sets the JS files that should be marked as already registered.
*
* @param string[] $keys
*/
public function setRegisteredJsFiles(array $keys)
{
$this->_registeredJsFiles = array_flip($keys);
}

/**
* Sets the asset bundle names that should be marked as already registered.
*
* @param string[] $names Asset bundle names
*/
public function setRegisteredAssetBundles(array $names)
{
$this->_registeredAssetBundles = array_flip($names);
}

// Events
// -------------------------------------------------------------------------

Expand Down Expand Up @@ -1335,6 +1384,28 @@ protected function renderBodyEndHtml($ajaxMode)
$lines[] = implode("\n", $this->_scripts[self::POS_END]);
}

if (Craft::$app->getRequest()->getIsCpRequest()) {
if (!empty($this->_registeredJsFiles)) {
$json = Json::encode($this->_registeredJsFiles);
$js = <<<JS
if (typeof Craft !== 'undefined') {
jQuery.extend(Craft.registeredJsFiles, {$json});
}
JS;
$this->registerJs($js, self::POS_END);
}

if (!empty($this->_registeredAssetBundles)) {
$json = Json::encode($this->_registeredAssetBundles);
$js = <<<JS
if (typeof Craft !== 'undefined') {
jQuery.extend(Craft.registeredAssetBundles, {$json});
}
JS;
$this->registerJs($js, self::POS_END);
}
}

$html = parent::renderBodyEndHtml($ajaxMode);

return empty($lines) ? $html : implode("\n", $lines).$html;
Expand Down Expand Up @@ -1376,6 +1447,19 @@ protected function registerAllAssetFiles()
}
}

/**
* @inheritdoc
*/
protected function registerAssetFiles($name)
{
// Don't re-register bundles
if (isset($this->_registeredAssetBundles[$name])) {
return;
}
$this->_registeredAssetBundles[$name] = true;
parent::registerAssetFiles($name);
}

// Private Methods
// =========================================================================

Expand Down
2 changes: 2 additions & 0 deletions src/web/assets/cp/CpAsset.php
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ private function _craftData(): array
'primarySiteId' => $isInstalled && !$isMigrationNeeded ? $sitesService->getPrimarySite()->id : null,
'Pro' => Craft::Pro,
'publishableSections' => $isInstalled && $currentUser ? $this->_publishableSections($currentUser) : [],
'registeredAssetBundles' => ['' => ''], // force encode as JS object
'registeredJsFiles' => ['' => ''], // force encode as JS object
'remainingSessionTime' => !in_array($request->getSegment(1), ['updates', 'manualupdate'], true) ? $userService->getRemainingSessionTime() : 0,
'right' => $orientation === 'ltr' ? 'right' : 'left',
'runQueueAutomatically' => (bool)$generalConfig->runQueueAutomatically,
Expand Down
7 changes: 5 additions & 2 deletions src/web/assets/cp/dist/js/Craft.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*! - 2018-05-30 */
/*! - 2018-06-01 */
(function($){

/** global: Craft */
Expand Down Expand Up @@ -340,7 +340,10 @@ $.extend(Craft,
data = {};
}

var headers = {};
var headers = {
'X-Registered-Asset-Bundles': Object.keys(Craft.registeredAssetBundles).join(','),
'X-Registered-Js-Files': Object.keys(Craft.registeredJsFiles).join(',')
};

if (Craft.csrfTokenValue && Craft.csrfTokenName) {
headers['X-CSRF-Token'] = Craft.csrfTokenValue;
Expand Down
2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/js/Craft.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/js/Craft.min.js.map

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion src/web/assets/cp/src/js/Craft.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,10 @@ $.extend(Craft,
data = {};
}

var headers = {};
var headers = {
'X-Registered-Asset-Bundles': Object.keys(Craft.registeredAssetBundles).join(','),
'X-Registered-Js-Files': Object.keys(Craft.registeredJsFiles).join(',')
};

if (Craft.csrfTokenValue && Craft.csrfTokenName) {
headers['X-CSRF-Token'] = Craft.csrfTokenValue;
Expand Down

0 comments on commit 6b446e8

Please sign in to comment.