Skip to content

Commit

Permalink
fix: dont initialise entity store on every render by using state [SPA…
Browse files Browse the repository at this point in the history
…-1711] (#208)

* fix: dont initialise entity store on every render by using state

* test: adjust the test to not pass a fake ref to the component
  • Loading branch information
Chaoste authored Dec 20, 2023
1 parent 5998d4d commit b04f44b
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('VisualEditorBlock', () => {
node={mockCompositionComponentNode}
dataSource={{}}
areEntitiesFetched={true}
entityStore={{ current: {} as EntityStore }}
entityStore={{} as EntityStore}
unboundValues={mockCompositionComponentNode.data.unboundValues}
resolveDesignValue={jest.fn()}
/>
Expand All @@ -109,7 +109,7 @@ describe('VisualEditorBlock', () => {
node={mockCompositionComponentNode}
dataSource={{}}
areEntitiesFetched={true}
entityStore={{ current: {} as EntityStore }}
entityStore={{} as EntityStore}
unboundValues={mockCompositionComponentNode.data.unboundValues}
resolveDesignValue={jest.fn()}
/>
Expand Down Expand Up @@ -138,7 +138,7 @@ describe('VisualEditorBlock', () => {
node={sectionNode}
dataSource={{}}
areEntitiesFetched={true}
entityStore={{ current: {} as EntityStore }}
entityStore={{} as EntityStore}
unboundValues={sectionNode.data.unboundValues}
resolveDesignValue={jest.fn()}
/>
Expand Down Expand Up @@ -166,7 +166,7 @@ describe('VisualEditorBlock', () => {
node={containerNode}
dataSource={{}}
areEntitiesFetched={true}
entityStore={{ current: {} as EntityStore }}
entityStore={{} as EntityStore}
unboundValues={containerNode.data.unboundValues}
resolveDesignValue={jest.fn()}
/>
Expand Down Expand Up @@ -213,7 +213,7 @@ describe('VisualEditorBlock', () => {
node={designComponentNode}
dataSource={{}}
areEntitiesFetched={true}
entityStore={{ current: entityStore }}
entityStore={entityStore}
unboundValues={designComponentNode.data.unboundValues}
resolveDesignValue={jest.fn()}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { RefObject, useCallback, useMemo } from 'react';
import React, { useCallback, useMemo } from 'react';

import type { EntityStore } from '@contentful/visual-sdk';
import omit from 'lodash.omit';
Expand Down Expand Up @@ -49,7 +49,7 @@ type VisualEditorBlockProps = {
unboundValues: CompositionUnboundValues;

resolveDesignValue: ResolveDesignValueType;
entityStore: RefObject<EntityStore>;
entityStore: EntityStore;
areEntitiesFetched: boolean;
};

Expand All @@ -65,7 +65,7 @@ export const VisualEditorBlock = ({
if (rawNode.type === DESIGN_COMPONENT_NODE_TYPE && areEntitiesFetched) {
return resolveDesignComponent({
node: rawNode,
entityStore: entityStore.current,
entityStore,
});
}

Expand Down Expand Up @@ -126,7 +126,7 @@ export const VisualEditorBlock = ({
const binding = dataSource[uuid] as Link<'Entry' | 'Asset'>;

let boundValue: string | Link<'Asset'> | undefined = areEntitiesFetched
? entityStore.current?.getValue(binding, path.slice(0, -1))
? entityStore.getValue(binding, path.slice(0, -1))
: undefined;

// In some cases, there may be an asset linked in the path, so we need to consider this scenario:
Expand All @@ -135,7 +135,7 @@ export const VisualEditorBlock = ({

if (!boundValue) {
const maybeBoundAsset = areEntitiesFetched
? entityStore.current?.getValue(binding, path.slice(0, -2))
? entityStore.getValue(binding, path.slice(0, -2))
: undefined;

if (isLinkToAsset(maybeBoundAsset)) {
Expand All @@ -144,7 +144,7 @@ export const VisualEditorBlock = ({
}

if (typeof boundValue === 'object' && boundValue.sys.linkType === 'Asset') {
boundValue = entityStore.current?.getValue(boundValue, ['fields', 'file']);
boundValue = entityStore.getValue(boundValue, ['fields', 'file']);
}

const value = boundValue || variableDefinition.defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type VisualEditorContextType = {
setSelectedNodeId: (id: string) => void;
unboundValues: CompositionUnboundValues;
breakpoints: Breakpoint[];
entityStore: React.MutableRefObject<EditorModeEntityStore>;
entityStore: EditorModeEntityStore;
areEntitiesFetched: boolean;
};

Expand All @@ -52,7 +52,7 @@ export const VisualEditorContext = React.createContext<VisualEditorContextType>(
},
locale: null,
breakpoints: [],
entityStore: {} as React.MutableRefObject<EditorModeEntityStore>,
entityStore: {} as EditorModeEntityStore,
areEntitiesFetched: false,
});

Expand Down Expand Up @@ -85,44 +85,47 @@ export function VisualEditorContextProvider({
const [locale, setLocale] = useState<string>(initialLocale);
const [areEntitiesFetched, setEntitiesFetched] = useState(false);

const entityStore = useRef<EditorModeEntityStore>(
new EditorModeEntityStore({
// Initially, the SDK boots in preview mode and already fetches entities.
// We utilizes the previosuly existing store to avoid loading twice and
// have the entities ready as early as possible.
entities: previousEntityStore?.entities ?? [],
locale: locale,
})
const [entityStore, setEntityStore] = useState<EditorModeEntityStore>(
() =>
new EditorModeEntityStore({
// Initially, the SDK boots in preview mode and already fetches entities.
// We utilizes the previosuly existing store to avoid loading twice and
// have the entities ready as early as possible.
entities: previousEntityStore?.entities ?? [],
locale,
})
);

// Reload the entity store when the locale changed
useEffect(() => {
const storeLocale = entityStore.current.locale;
const storeLocale = entityStore.locale;
if (!locale || locale === storeLocale) return;
console.debug(
`[exp-builder.sdk] Resetting entity store because the locale changed from '${storeLocale}' to '${locale}'.`
);
entityStore.current = new EditorModeEntityStore({
entities: [],
locale: locale,
});
}, [locale]);
setEntityStore(
new EditorModeEntityStore({
entities: [],
locale: locale,
})
);
}, [entityStore, locale]);

// When the tree was updated, we store the dataSource and
// afterward, this effect fetches the respective entities.
useEffect(() => {
const resolveEntities = async () => {
setEntitiesFetched(false);
const dataSourceEntityLinks = Object.values(dataSource || {});
await entityStore.current.fetchEntities([
await entityStore.fetchEntities([
...dataSourceEntityLinks,
...(designComponentsRegistry.values() || []),
]);
setEntitiesFetched(true);
};

resolveEntities();
}, [dataSource]);
}, [dataSource, entityStore]);

const reloadApp = () => {
sendMessage(OUTGOING_EVENTS.CanvasReload, {});
Expand Down Expand Up @@ -252,7 +255,7 @@ export function VisualEditorContextProvider({
designComponent: Entry;
designComponentDefinition?: ComponentRegistration['definition'];
} = payload;
entityStore.current.updateEntity(designComponent);
entityStore.updateEntity(designComponent);
// Using a Map here to avoid setting state and rerending all existing design components when a new design component is added
// TODO: Figure out if we can extend this love to data source and unbound values. Maybe that'll solve the blink
// of all bound and unbound values when new values are added
Expand Down Expand Up @@ -286,7 +289,7 @@ export function VisualEditorContextProvider({
}
case INCOMING_EVENTS.UpdatedEntity: {
const { entity } = payload;
entity && entityStore.current.updateEntity(entity);
entity && entityStore.updateEntity(entity);
break;
}
case INCOMING_EVENTS.RequestEditorMode: {
Expand All @@ -305,7 +308,7 @@ export function VisualEditorContextProvider({
return () => {
window.removeEventListener('message', onMessage);
};
}, [mode]);
}, [entityStore, mode]);

/*
* Handles on scroll business
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ export class EditorModeEntityStore extends EditorEntityStore {
}

async fetchEntities(entityLinks: UnresolvedLink<'Entry' | 'Asset'>[]) {
const entryLinks = entityLinks.filter((link) => link.sys.linkType === 'Entry');
const assetLinks = entityLinks.filter((link) => link.sys.linkType === 'Asset');
const entryLinks = entityLinks.filter((link) => link.sys?.linkType === 'Entry');
const assetLinks = entityLinks.filter((link) => link.sys?.linkType === 'Asset');

const uniqueEntryLinks = new Set(entryLinks.map((link) => link.sys.id));
const uniqueAssetLinks = new Set(assetLinks.map((link) => link.sys.id));
Expand Down

0 comments on commit b04f44b

Please sign in to comment.