From 81b90d4688ea38f42bc8e96cdc6dda0beb046ccb Mon Sep 17 00:00:00 2001 From: Tomas Cerskus Date: Wed, 15 May 2024 17:09:55 +0100 Subject: [PATCH 1/2] feature: SSR support Before these changes, the Frame and Element components delayed rendering user components until after the first render due to their reliance on useEffect. This meant that (a) server-side rendering did not work (b) pages with many deeply nested components took many render interations to fully render a page which on slower devices would cause a visible layout shift. After this change, Element no longer relies on useEffect and Frame, no longer relies on useEffect if initialData is provided. --- .changeset/real-glasses-cross.md | 5 +++++ packages/core/src/nodes/Element.tsx | 10 ++++------ .../core/src/nodes/tests/Element.test.tsx | 4 +++- packages/core/src/render/Frame.tsx | 20 +++++++++---------- 4 files changed, 22 insertions(+), 17 deletions(-) create mode 100644 .changeset/real-glasses-cross.md diff --git a/.changeset/real-glasses-cross.md b/.changeset/real-glasses-cross.md new file mode 100644 index 000000000..279c898a8 --- /dev/null +++ b/.changeset/real-glasses-cross.md @@ -0,0 +1,5 @@ +--- +'@craftjs/core': patch +--- + +SSR support diff --git a/packages/core/src/nodes/Element.tsx b/packages/core/src/nodes/Element.tsx index b53196f93..8db7f4f6e 100644 --- a/packages/core/src/nodes/Element.tsx +++ b/packages/core/src/nodes/Element.tsx @@ -1,4 +1,4 @@ -import { ERROR_TOP_LEVEL_ELEMENT_NO_ID, useEffectOnce } from '@craftjs/utils'; +import { ERROR_TOP_LEVEL_ELEMENT_NO_ID } from '@craftjs/utils'; import React, { useState } from 'react'; import invariant from 'tiny-invariant'; @@ -47,9 +47,7 @@ export function Element({ }, })); - const [linkedNodeId, setLinkedNodeId] = useState(null); - - useEffectOnce(() => { + const [linkedNodeId] = useState(() => { invariant(!!id, ERROR_TOP_LEVEL_ELEMENT_NO_ID); const { id: nodeId, data } = node; @@ -77,9 +75,9 @@ export function Element({ linkedNodeId = tree.rootNodeId; actions.history.ignore().addLinkedNodeFromTree(tree, nodeId, id); } - - setLinkedNodeId(linkedNodeId); + return linkedNodeId; } + return null; }); return linkedNodeId ? : null; diff --git a/packages/core/src/nodes/tests/Element.test.tsx b/packages/core/src/nodes/tests/Element.test.tsx index ea8401693..a1d72f1b3 100644 --- a/packages/core/src/nodes/tests/Element.test.tsx +++ b/packages/core/src/nodes/tests/Element.test.tsx @@ -79,7 +79,9 @@ describe('', () => { }); it('should call query.parseReactElement()', () => { - expect(parseReactElement).toHaveBeenCalledWith( + const arg0 = parseReactElement.mock.calls[0][0]; + const arg0WithoutOwner = { ...arg0, _owner: null }; + expect(arg0WithoutOwner).toEqual( {children} ); }); diff --git a/packages/core/src/render/Frame.tsx b/packages/core/src/render/Frame.tsx index 5679850bf..214ddab6a 100644 --- a/packages/core/src/render/Frame.tsx +++ b/packages/core/src/render/Frame.tsx @@ -44,21 +44,21 @@ export const Frame: React.FC> = ({ initialData: data || json, }); - const isInitialChildrenLoadedRef = useRef(false); + const isInitialLoadedRef = useRef(false); - useEffect(() => { - const { initialChildren, initialData } = initialState.current; + if (!isInitialLoadedRef.current && initialState.current.initialData) { + actions.history.ignore().deserialize(initialState.current.initialData); + isInitialLoadedRef.current = true; + } - if (initialData) { - actions.history.ignore().deserialize(initialData); - return; - } + useEffect(() => { + const { initialChildren } = initialState.current; // Prevent recreating Nodes from child elements if we already did it the first time // Usually an issue in React Strict Mode where this hook is called twice which results in orphaned Nodes - const isInitialChildrenLoaded = isInitialChildrenLoadedRef.current; + const isInitialLoaded = isInitialLoadedRef.current; - if (!initialChildren || isInitialChildrenLoaded) { + if (!initialChildren || isInitialLoaded) { return; } @@ -72,7 +72,7 @@ export const Frame: React.FC> = ({ }); actions.history.ignore().addNodeTree(node); - isInitialChildrenLoadedRef.current = true; + isInitialLoadedRef.current = true; }, [actions, query]); return ; From e4c6692e296563d44e6258d2f0592af9f4381a15 Mon Sep 17 00:00:00 2001 From: Tomas Cerskus Date: Thu, 16 May 2024 18:06:37 +0100 Subject: [PATCH 2/2] Completely get rid of useEffect in Frame component --- packages/core/src/render/Frame.tsx | 47 +++++++++++------------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/packages/core/src/render/Frame.tsx b/packages/core/src/render/Frame.tsx index 214ddab6a..96e1813da 100644 --- a/packages/core/src/render/Frame.tsx +++ b/packages/core/src/render/Frame.tsx @@ -1,5 +1,5 @@ import { deprecationWarning, ROOT_NODE } from '@craftjs/utils'; -import React, { useEffect, useRef } from 'react'; +import React, { useRef } from 'react'; import { useInternalEditor } from '../editor/useInternalEditor'; import { SerializedNodes } from '../interfaces'; @@ -39,41 +39,28 @@ export const Frame: React.FC> = ({ }); } - const initialState = useRef({ - initialChildren: children, - initialData: data || json, - }); + const isLoaded = useRef(false); - const isInitialLoadedRef = useRef(false); + if (!isLoaded.current) { + const initialData = data || json; - if (!isInitialLoadedRef.current && initialState.current.initialData) { - actions.history.ignore().deserialize(initialState.current.initialData); - isInitialLoadedRef.current = true; - } - - useEffect(() => { - const { initialChildren } = initialState.current; + if (initialData) { + actions.history.ignore().deserialize(initialData); + } else { + const rootNode = React.Children.only(children) as React.ReactElement; - // Prevent recreating Nodes from child elements if we already did it the first time - // Usually an issue in React Strict Mode where this hook is called twice which results in orphaned Nodes - const isInitialLoaded = isInitialLoadedRef.current; + const node = query.parseReactElement(rootNode).toNodeTree((node, jsx) => { + if (jsx === rootNode) { + node.id = ROOT_NODE; + } + return node; + }); - if (!initialChildren || isInitialLoaded) { - return; + actions.history.ignore().addNodeTree(node); } - const rootNode = React.Children.only(initialChildren) as React.ReactElement; - - const node = query.parseReactElement(rootNode).toNodeTree((node, jsx) => { - if (jsx === rootNode) { - node.id = ROOT_NODE; - } - return node; - }); - - actions.history.ignore().addNodeTree(node); - isInitialLoadedRef.current = true; - }, [actions, query]); + isLoaded.current = true; + } return ; };