Merge pull request #3770 from neos/bugfix/remove-ui-script-tag

BUGFIX: Improve UI loading performance by removing UI nodedata script tag and using less data
neos · Aug 12, 2024 · ece6ece · ece6ece
2 parents 6d04033 + f4520af
commit ece6ece
Show file tree

Hide file tree

Showing 9 changed files with 146 additions and 102 deletions.
diff --git a/Classes/Aspects/AugmentationAspect.php b/Classes/Aspects/AugmentationAspect.php
@@ -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
@@ -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);
     }
 
     /**

diff --git a/Classes/ContentRepository/Service/NodeService.php b/Classes/ContentRepository/Service/NodeService.php
@@ -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;
@@ -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
      *
@@ -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];
 
-        $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);

diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php
@@ -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'));
+
         $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) {
@@ -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;
         }
 

diff --git a/Classes/Domain/Service/UserLocaleService.php b/Classes/Domain/Service/UserLocaleService.php
@@ -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
 {
@@ -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
      */
@@ -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);
     }
 }
diff --git a/Classes/FlowQueryOperations/NeosUiDefaultNodesOperation.php b/Classes/FlowQueryOperations/NeosUiDefaultNodesOperation.php
@@ -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;
 

diff --git a/Classes/Fusion/Helper/NodeInfoHelper.php b/Classes/Fusion/Helper/NodeInfoHelper.php
@@ -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)
@@ -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);
@@ -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()
             ];
         };
 

diff --git a/packages/neos-ui-guest-frame/src/initializeContentDomNode.js b/packages/neos-ui-guest-frame/src/initializeContentDomNode.js
@@ -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]) {

diff --git a/packages/neos-ui-guest-frame/src/initializeGuestFrame.js b/packages/neos-ui-guest-frame/src/initializeGuestFrame.js
@@ -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.
@@ -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();
@@ -150,8 +178,7 @@ export default ({globalRegistry, store}) => function * initializeGuestFrame() {
             store,
             globalRegistry,
             nodeTypesRegistry,
-            inlineEditorRegistry,
-            nodes
+            inlineEditorRegistry
         });
 
         requestIdleCallback(() => {