Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Improve UI loading performance by removing UI nodedata script tag and using less data #3770

Merged
merged 13 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 1 addition & 22 deletions Classes/Aspects/AugmentationAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,12 @@ class AugmentationAspect
*/
protected $nodeAuthorizationService;

/**
* @Flow\Inject
* @var UserLocaleService
*/
protected $userLocaleService;

/**
* @Flow\Inject
* @var HtmlAugmenter
*/
protected $htmlAugmenter;

/**
* @Flow\Inject
* @var NodeInfoHelper
*/
protected $nodeInfoHelper;

/**
* @Flow\Inject
* @var SessionInterface
Expand Down Expand Up @@ -126,16 +114,7 @@ public function contentElementAugmentation(JoinPointInterface $joinPoint)
$attributes['data-__neos-node-contextpath'] = $node->getContextPath();
$attributes['data-__neos-fusion-path'] = $fusionPath;

$this->userLocaleService->switchToUILocale();

$serializedNode = json_encode($this->nodeInfoHelper->renderNodeWithPropertiesAndChildrenInformation($node, $this->controllerContext));

$this->userLocaleService->switchToUILocale(true);

$wrappedContent = $this->htmlAugmenter->addAttributes($content, $attributes, 'div');
$wrappedContent .= "<script data-neos-nodedata>(function(){(this['@Neos.Neos.Ui:Nodes'] = this['@Neos.Neos.Ui:Nodes'] || {})['{$node->getContextPath()}'] = {$serializedNode}})()</script>";

return $wrappedContent;
return $this->htmlAugmenter->addAttributes($content, $attributes);
}

/**
Expand Down
69 changes: 41 additions & 28 deletions Classes/ContentRepository/Service/NodeService.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Neos\ContentRepository\Domain\Model\NodeInterface;
use Neos\ContentRepository\Domain\Model\Workspace;
use Neos\ContentRepository\Domain\Service\Context;
use Neos\ContentRepository\Domain\Service\ContextFactoryInterface;
use Neos\ContentRepository\Domain\Utility\NodePaths;
use Neos\Eel\FlowQuery\FlowQuery;
Expand Down Expand Up @@ -46,6 +47,11 @@ class NodeService
*/
protected $domainRepository;

/**
* @var array<string, Context>
*/
protected array $contextCache = [];

/**
* Helper method to retrieve the closest document for a node
*
Expand Down Expand Up @@ -86,36 +92,43 @@ public function getNodeFromContextPath($contextPath, Site $site = null, Domain $
$nodePath = $nodePathAndContext['nodePath'];
$workspaceName = $nodePathAndContext['workspaceName'];
$dimensions = $nodePathAndContext['dimensions'];
$siteNodeName = $site ? $site->getNodeName() : explode('/', $nodePath)[2];
Sebobo marked this conversation as resolved.
Show resolved Hide resolved

$contextProperties = $this->prepareContextProperties($workspaceName, $dimensions);

if ($site === null) {
list(, , $siteNodeName) = explode('/', $nodePath);
$site = $this->siteRepository->findOneByNodeName($siteNodeName);
}

if ($domain === null) {
$domain = $this->domainRepository->findOneBySite($site);
}

$contextProperties['currentSite'] = $site;
$contextProperties['currentDomain'] = $domain;
if ($includeAll === true) {
$contextProperties['invisibleContentShown'] = true;
$contextProperties['removedContentShown'] = true;
$contextProperties['inaccessibleContentShown'] = true;
}

$context = $this->contextFactory->create(
$contextProperties
);

$workspace = $context->getWorkspace(false);
if (!$workspace) {
return new Error(
sprintf('Could not convert the given source to Node object because the workspace "%s" as specified in the context node path does not exist.', $workspaceName),
1451392329
// Prevent reloading the same context multiple times
$contextHash = md5(implode('|', [$siteNodeName, $workspaceName, json_encode($dimensions), $includeAll]));
if (isset($this->contextCache[$contextHash])) {
$context = $this->contextCache[$contextHash];
} else {
$contextProperties = $this->prepareContextProperties($workspaceName, $dimensions);

if ($site === null) {
$site = $this->siteRepository->findOneByNodeName($siteNodeName);
}

if ($domain === null) {
$domain = $this->domainRepository->findOneBySite($site);
}

$contextProperties['currentSite'] = $site;
$contextProperties['currentDomain'] = $domain;
if ($includeAll === true) {
$contextProperties['invisibleContentShown'] = true;
$contextProperties['removedContentShown'] = true;
$contextProperties['inaccessibleContentShown'] = true;
}

$context = $this->contextFactory->create(
$contextProperties
);

$workspace = $context->getWorkspace(false);
if (!$workspace) {
return new Error(
sprintf('Could not convert the given source to Node object because the workspace "%s" as specified in the context node path does not exist.', $workspaceName),
1451392329
);
}
$this->contextCache[$contextHash] = $context;
}

return $context->getNode($nodePath);
Expand Down
26 changes: 20 additions & 6 deletions Classes/Controller/BackendServiceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -532,11 +532,14 @@ public function flowQueryAction(array $chain): string
$createContext = array_shift($chain);
$finisher = array_pop($chain);

// we deduplicate passed nodes here
$nodeContextPaths = array_unique(array_column($createContext['payload'], '$node'));
Sebobo marked this conversation as resolved.
Show resolved Hide resolved

$flowQuery = new FlowQuery(array_map(
function ($envelope) {
return $this->nodeService->getNodeFromContextPath($envelope['$node']);
function ($contextPath) {
return $this->nodeService->getNodeFromContextPath($contextPath);
},
$createContext['payload']
$nodeContextPaths
));

foreach ($chain as $operation) {
Expand All @@ -548,14 +551,25 @@ function ($envelope) {

switch ($finisher['type']) {
case 'get':
$result = $nodeInfoHelper->renderNodes(array_filter($flowQuery->get()), $this->getControllerContext());
$result = $nodeInfoHelper->renderNodes(
array_filter($flowQuery->get()),
$this->getControllerContext()
);
break;
case 'getForTree':
$result = $nodeInfoHelper->renderNodes(array_filter($flowQuery->get()), $this->getControllerContext(), true);
$result = $nodeInfoHelper->renderNodes(
array_filter($flowQuery->get()),
$this->getControllerContext(),
true
);
break;
case 'getForTreeWithParents':
$nodeTypeFilter = $finisher['payload']['nodeTypeFilter'] ?? null;
$result = $nodeInfoHelper->renderNodesWithParents(array_filter($flowQuery->get()), $this->getControllerContext(), $nodeTypeFilter);
$result = $nodeInfoHelper->renderNodesWithParents(
array_filter($flowQuery->get()),
$this->getControllerContext(),
$nodeTypeFilter
);
break;
}

Expand Down
28 changes: 22 additions & 6 deletions Classes/Domain/Service/UserLocaleService.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Neos\Flow\I18n\Locale;
use Neos\Flow\I18n\Service as I18nService;
use Neos\Neos\Domain\Service\UserService;
use Neos\Neos\Fusion\Helper\NodeLabelToken;

class UserLocaleService
{
Expand All @@ -37,8 +38,18 @@ class UserLocaleService
*/
protected $rememberedContentLocale;

/**
* The current user's locale (cached for performance)
*
* @var Locale
*/
protected $userLocaleRuntimeCache;

/**
* For serialization, we need to respect the UI locale, rather than the content locale
* This is done to translate the node labels correctly.
* For example {@see NodeLabelToken::resolveLabelFromNodeType()} will call the translator which will uses the globally set locale.
* FIXME we should eliminate hacking the global state and passing the locale differently
*
* @param boolean $reset Reset to remebered locale
*/
Expand All @@ -47,12 +58,17 @@ public function switchToUILocale($reset = false)
if ($reset === true) {
// Reset the locale
$this->i18nService->getConfiguration()->setCurrentLocale($this->rememberedContentLocale);
} else {
$this->rememberedContentLocale = $this->i18nService->getConfiguration()->getCurrentLocale();
$userLocalePreference = ($this->userService->getCurrentUser() ? $this->userService->getCurrentUser()->getPreferences()->getInterfaceLanguage() : null);
$defaultLocale = $this->i18nService->getConfiguration()->getDefaultLocale();
$userLocale = $userLocalePreference ? new Locale($userLocalePreference) : $defaultLocale;
$this->i18nService->getConfiguration()->setCurrentLocale($userLocale);
return;
}
$this->rememberedContentLocale = $this->i18nService->getConfiguration()->getCurrentLocale();
if ($this->userLocaleRuntimeCache) {
$this->i18nService->getConfiguration()->setCurrentLocale($this->userLocaleRuntimeCache);
return;
}
$userLocalePreference = ($this->userService->getCurrentUser() ? $this->userService->getCurrentUser()->getPreferences()->getInterfaceLanguage() : null);
$defaultLocale = $this->i18nService->getConfiguration()->getDefaultLocale();
$userLocale = $userLocalePreference ? new Locale($userLocalePreference) : $defaultLocale;
$this->userLocaleRuntimeCache = $userLocale;
$this->i18nService->getConfiguration()->setCurrentLocale($userLocale);
}
}
3 changes: 2 additions & 1 deletion Classes/FlowQueryOperations/NeosUiDefaultNodesOperation.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ public function canEvaluate($context)
public function evaluate(FlowQuery $flowQuery, array $arguments)
{
/** @var TraversableNodeInterface $siteNode */
$siteNode = $flowQuery->getContext()[0];
/** @var TraversableNodeInterface $documentNode */
list($siteNode, $documentNode) = $flowQuery->getContext();
$documentNode = $flowQuery->getContext()[1] ?? $siteNode;
/** @var string[] $toggledNodes */
list($baseNodeType, $loadingDepth, $toggledNodes, $clipboardNodesContextPaths) = $arguments;

Expand Down
8 changes: 4 additions & 4 deletions Classes/Fusion/Helper/NodeInfoHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public function renderNodeWithMinimalPropertiesAndChildrenInformation(NodeInterf
/**
* @param NodeInterface $node
* @param ControllerContext|null $controllerContext
* @param string $nodeTypeFilterOverride
* @param string|null $nodeTypeFilterOverride
* @return array|null
*/
public function renderNodeWithPropertiesAndChildrenInformation(NodeInterface $node, ControllerContext $controllerContext = null, string $nodeTypeFilterOverride = null)
Expand All @@ -177,7 +177,7 @@ public function renderNodeWithPropertiesAndChildrenInformation(NodeInterface $no
}
}

$baseNodeType = $nodeTypeFilterOverride ? $nodeTypeFilterOverride : (isset($presetBaseNodeType) ? $presetBaseNodeType : $this->defaultBaseNodeType);
$baseNodeType = $nodeTypeFilterOverride ?: (isset($presetBaseNodeType) ? $presetBaseNodeType : $this->defaultBaseNodeType);
$nodeInfo['children'] = $this->renderChildrenInformation($node, $baseNodeType);

$this->userLocaleService->switchToUILocale(true);
Expand Down Expand Up @@ -246,10 +246,10 @@ protected function renderChildrenInformation(NodeInterface $node, string $nodeTy
$contentChildNodes = $node->getChildNodes($this->buildContentChildNodeFilterString());
$childNodes = array_merge($documentChildNodes, $contentChildNodes);

$mapper = function (NodeInterface $childNode) {
$mapper = static function (NodeInterface $childNode) {
return [
'contextPath' => $childNode->getContextPath(),
'nodeType' => $childNode->getNodeType()->getName() // TODO: DUPLICATED; should NOT be needed!!!
'nodeType' => $childNode->getNodeType()->getName()
];
};

Expand Down
3 changes: 2 additions & 1 deletion packages/neos-ui-guest-frame/src/initializeContentDomNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import initializePropertyDomNode from './initializePropertyDomNode';

import style from './style.module.css';

export default ({store, globalRegistry, nodeTypesRegistry, inlineEditorRegistry, nodes}) => contentDomNode => {
export default ({store, globalRegistry, nodeTypesRegistry, inlineEditorRegistry}) => contentDomNode => {
const nodes = store.getState().cr.nodes.byContextPath;
const contextPath = contentDomNode.getAttribute('data-__neos-node-contextpath');

if (!nodes[contextPath]) {
Expand Down
47 changes: 37 additions & 10 deletions packages/neos-ui-guest-frame/src/initializeGuestFrame.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
import {takeEvery, put, select} from 'redux-saga/effects';
import {put, select, takeEvery} from 'redux-saga/effects';
import {$get} from 'plow-js';

import {selectors, actions, actionTypes} from '@neos-project/neos-ui-redux-store';
import {actions, actionTypes, selectors} from '@neos-project/neos-ui-redux-store';
import {requestIdleCallback} from '@neos-project/utils-helpers';

import initializeContentDomNode from './initializeContentDomNode';
import {
getGuestFrameWindow,
getGuestFrameDocument,
dispatchCustomEvent,
findAllNodesInGuestFrame,
findInGuestFrame,
findNodeInGuestFrame,
dispatchCustomEvent
getGuestFrameDocument,
getGuestFrameWindow
} from './dom';

import style from './style.module.css';
import {SelectionModeTypes} from '@neos-project/neos-ts-interfaces';
import backend from '@neos-project/neos-ui-backend-connector';

//
// Get all parent elements of the event target.
Expand Down Expand Up @@ -60,13 +61,40 @@ export default ({globalRegistry, store}) => function * initializeGuestFrame() {
return;
}

const nodes = Object.assign({}, guestFrameWindow['@Neos.Neos.Ui:Nodes'], {
[documentInformation.metaData.documentNode]: documentInformation.metaData.documentNodeSerialization
// Load legacy node data scripts from guest frame - remove with Neos 9.0
const legacyNodeData = guestFrameWindow['@Neos.Neos.Ui:Nodes'] || {};

// Load all nodedata for nodes in the guest frame and filter duplicates
const {q} = yield backend.get();
const nodeContextPathsInGuestFrame = findAllNodesInGuestFrame().map(node => node.getAttribute('data-__neos-node-contextpath'));

// Filter nodes that are already present in the redux store and duplicates
const nodesByContextPath = store.getState().cr.nodes.byContextPath;
const notFullyLoadedNodeContextPaths = [...new Set(nodeContextPathsInGuestFrame)].filter((contextPath) => {
const node = nodesByContextPath[contextPath];
const nodeIsLoaded = node !== undefined && node.isFullyLoaded;
return !nodeIsLoaded;
});

// Load remaining list of not fully loaded nodes from the backend if there are any
const fullyLoadedNodesFromContent = notFullyLoadedNodeContextPaths.length > 0 ? (yield q(notFullyLoadedNodeContextPaths).get()).reduce((nodes, node) => {
nodes[node.contextPath] = node;
return nodes;
}, {}) : {};

const nodes = Object.assign(
{},
legacyNodeData, // Merge legacy node data from the guest frame - remove with Neos 9.0
fullyLoadedNodesFromContent,
{
[documentInformation.metaData.documentNode]: documentInformation.metaData.documentNodeSerialization
}
);

// Merge new nodes into the store
yield put(actions.CR.Nodes.merge(nodes));

// Remove the inline scripts after initialization
// Remove the legacy inline scripts after initialization - remove with Neos 9.0
Array.prototype.forEach.call(guestFrameWindow.document.querySelectorAll('script[data-neos-nodedata]'), element => element.parentElement.removeChild(element));

const state = store.getState();
Expand Down Expand Up @@ -150,8 +178,7 @@ export default ({globalRegistry, store}) => function * initializeGuestFrame() {
store,
globalRegistry,
nodeTypesRegistry,
inlineEditorRegistry,
nodes
inlineEditorRegistry
});

requestIdleCallback(() => {
Expand Down
Loading
Loading