diff --git a/packages/container/src/components/FieldPluginContainer.tsx b/packages/container/src/components/FieldPluginContainer.tsx index e5a64868..bf2a418f 100644 --- a/packages/container/src/components/FieldPluginContainer.tsx +++ b/packages/container/src/components/FieldPluginContainer.tsx @@ -77,8 +77,9 @@ const useSandbox = ( return undefined } // Omitting query parameters from the user-provided URL in a safe way - return `${fieldPluginURL.origin}${fieldPluginURL.pathname - }?${urlSearchParamsFromPluginUrlParams(pluginParams)}` + return `${fieldPluginURL.origin}${ + fieldPluginURL.pathname + }?${urlSearchParamsFromPluginUrlParams(pluginParams)}` }, [fieldPluginURL, pluginParams]) const [iframeKey, setIframeKey] = useState(0) @@ -157,11 +158,12 @@ const useSandbox = ( ) const onContextRequested = useCallback( - () => + (callbackId: string) => dispatchContextRequest({ uid, action: 'get-context', story: loadedData.story, + callbackId, }), [uid, dispatchContextRequest, loadedData.story], ) diff --git a/packages/container/src/dom/createContainerMessageListener.ts b/packages/container/src/dom/createContainerMessageListener.ts index fedf8dad..040a5acc 100644 --- a/packages/container/src/dom/createContainerMessageListener.ts +++ b/packages/container/src/dom/createContainerMessageListener.ts @@ -7,7 +7,6 @@ import { isPluginLoadedMessage, isValueChangeMessage, PluginLoadedMessage, - RequestContext, SetContent, SetModalOpen, } from '@storyblok/field-plugin' @@ -17,7 +16,7 @@ type ContainerActions = { setContent: SetContent setModalOpen: SetModalOpen setPluginReady: (message: PluginLoadedMessage) => void - requestContext: RequestContext + requestContext: (callbackId: string) => void selectAsset: (callbackId: string, field: string) => void } @@ -57,7 +56,7 @@ export const createContainerMessageListener: CreateContainerListener = ( } else if (isAssetModalChangeMessage(message)) { eventHandlers.selectAsset(message.callbackId, message.field ?? '') } else if (isGetContextMessage(message)) { - eventHandlers.requestContext() + eventHandlers.requestContext(message.callbackId) } else { console.warn( `Container received unknown message from plugin: ${JSON.stringify( diff --git a/packages/field-plugin/src/createFieldPlugin/FieldPluginActions.ts b/packages/field-plugin/src/createFieldPlugin/FieldPluginActions.ts index 616b5dc2..19e59770 100644 --- a/packages/field-plugin/src/createFieldPlugin/FieldPluginActions.ts +++ b/packages/field-plugin/src/createFieldPlugin/FieldPluginActions.ts @@ -1,9 +1,9 @@ -import { Asset } from '../messaging' +import { Asset, ContextRequestMessage } from '../messaging' export type SetStateAction = T | ((value: T) => T) export type SetContent = (setContentAction: SetStateAction) => void export type SetModalOpen = (setModalOpenAction: SetStateAction) => void -export type RequestContext = () => void +export type RequestContext = () => Promise export type SelectAsset = () => Promise export type FieldPluginActions = { diff --git a/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.test.ts b/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.test.ts index ecc58898..49b350a3 100644 --- a/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.test.ts +++ b/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.test.ts @@ -2,7 +2,6 @@ import { createPluginActions } from './createPluginActions' import { AssetModalChangeMessage, GetContextMessage, - HeightChangeMessage, ModalChangeMessage, ValueChangeMessage, } from '../../messaging' @@ -139,6 +138,7 @@ describe('createPluginActions', () => { const { actions: { requestContext }, } = createPluginActions(uid, postToContainer, onUpdateState) + // eslint-disable-next-line @typescript-eslint/no-floating-promises requestContext() expect(postToContainer).toHaveBeenLastCalledWith( expect.objectContaining({ @@ -147,6 +147,55 @@ describe('createPluginActions', () => { ) }) }) + it('requestContext calls the callback function when the container answers', async () => { + const { uid, postToContainer, onUpdateState } = mock() + const { + actions: { requestContext }, + messageCallbacks: { onContextRequest }, + } = createPluginActions(uid, postToContainer, onUpdateState) + const promise = requestContext() + onContextRequest({ + uid, + story: { content: {} }, + action: 'get-context', + callbackId: TEST_CALLBACK_ID, + }) + const result = await promise + expect(result).toHaveProperty('callbackId', TEST_CALLBACK_ID) + }) + it('requestContext does not call the callback function when callbackId does not match', async () => { + const WRONG_CALLBACK_ID = TEST_CALLBACK_ID + '_wrong' + const { uid, postToContainer, onUpdateState } = mock() + const { + actions: { requestContext }, + messageCallbacks: { onContextRequest }, + } = createPluginActions(uid, postToContainer, onUpdateState) + const promise = requestContext() + onContextRequest({ + uid, + story: { content: {} }, + action: 'get-context', + callbackId: WRONG_CALLBACK_ID, + }) + const resolvedFn = jest.fn() + const rejectedFn = jest.fn() + promise.then(resolvedFn).catch(rejectedFn) + await wait(100) + expect(resolvedFn).toHaveBeenCalledTimes(0) + expect(rejectedFn).toHaveBeenCalledTimes(0) + }) + it('requestContext should reject the second request if the first one is not resolved yet', async () => { + const { uid, postToContainer, onUpdateState } = mock() + const { + actions: { requestContext }, + } = createPluginActions(uid, postToContainer, onUpdateState) + // eslint-disable-next-line @typescript-eslint/no-floating-promises + requestContext() + const promise2 = requestContext() + await expect(promise2).rejects.toMatchInlineSnapshot( + `"Please wait until a previous requestContext call is resolved."`, + ) + }) describe('selectAsset()', () => { it('send a message to the container to open the asset selector', () => { const { uid, postToContainer, onUpdateState } = mock() @@ -161,7 +210,7 @@ describe('createPluginActions', () => { } satisfies Partial), ) }) - it('calls the callback function when an asset has been selected by the user', async () => { + it('selectAsset calls the callback function when an asset has been selected by the user', async () => { const { uid, postToContainer, onUpdateState } = mock() const { actions: { selectAsset }, @@ -179,7 +228,7 @@ describe('createPluginActions', () => { const result = await promise expect(result).toEqual({ filename }) }) - it('does not call the callack function when callbackId does not match', async () => { + it('selectAsset does not call the callback function when callbackId does not match', async () => { const WRONG_CALLBACK_ID = TEST_CALLBACK_ID + '_wrong' const { uid, postToContainer, onUpdateState } = mock() const { @@ -202,7 +251,7 @@ describe('createPluginActions', () => { expect(resolvedFn).toHaveBeenCalledTimes(0) expect(rejectedFn).toHaveBeenCalledTimes(0) }) - it('should reject the second selectAsset request if the first one is not resolved yet', async () => { + it('selectAsset should reject the second request if the first one is not resolved yet', async () => { const { uid, postToContainer, onUpdateState } = mock() const { actions: { selectAsset }, diff --git a/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.ts b/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.ts index 3efd2245..6957a705 100644 --- a/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.ts +++ b/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginActions.ts @@ -5,6 +5,7 @@ import { Asset, assetFromAssetSelectedMessage, assetModalChangeMessage, + ContextRequestMessage, getContextMessage, heightChangeMessage, modalChangeMessage, @@ -63,6 +64,11 @@ export const createPluginActions: CreatePluginActions = ( undefined let assetSelectedCallbackId: undefined | string = undefined + let requestContextCallbackRef: + | undefined + | ((message: ContextRequestMessage) => void) = undefined + let requestContextCallbackId: undefined | string = undefined + const onStateChange: OnStateChangeMessage = (data) => { state = { ...state, @@ -76,6 +82,17 @@ export const createPluginActions: CreatePluginActions = ( ...partialPluginStateFromContextRequestMessage(data), } onUpdateState(state) + + // We do not reject the promise here. + // There can be another instance of `createFieldPlugin()`, + // calling `requestContext` with different `callbackId`. + // In such case, we should simply ignore the callback. + // We may get another callback with correct `callbackId`. + if (data.callbackId === requestContextCallbackId) { + requestContextCallbackRef?.(data) + requestContextCallbackId = undefined + requestContextCallbackRef = undefined + } } const onAssetSelect: OnAssetSelectMessage = (data) => { // We do not reject the promise here. @@ -152,7 +169,20 @@ export const createPluginActions: CreatePluginActions = ( postToContainer(assetModalChangeMessage(uid, callbackId)) }) }, - requestContext: () => postToContainer(getContextMessage(uid)), + requestContext: () => { + if (requestContextCallbackId !== undefined) { + // eslint-disable-next-line functional/no-promise-reject + return Promise.reject( + 'Please wait until a previous requestContext call is resolved.', + ) + } + const callbackId = getRandomString(16) + requestContextCallbackId = callbackId + return new Promise((resolve) => { + requestContextCallbackRef = resolve + postToContainer(getContextMessage(uid, callbackId)) + }) + }, }, messageCallbacks, onHeightChange, diff --git a/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginMessageListener/handlePluginMessage.test.ts b/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginMessageListener/handlePluginMessage.test.ts index 01e708ae..d5d368df 100644 --- a/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginMessageListener/handlePluginMessage.test.ts +++ b/packages/field-plugin/src/createFieldPlugin/createPluginActions/createPluginMessageListener/handlePluginMessage.test.ts @@ -8,6 +8,8 @@ import { } from '../../../messaging' const uid = 'abc123' +const callbackId = 'test-callback-id' + const mockCallbacks = (): PluginMessageCallbacks => ({ onStateChange: jest.fn(), onContextRequest: jest.fn(), @@ -73,6 +75,7 @@ describe('handlePluginMessage', () => { const data: ContextRequestMessage = { action: 'get-context', uid, + callbackId, story: { content: {} }, } const callbacks = mockCallbacks() @@ -88,7 +91,7 @@ describe('handlePluginMessage', () => { uid, filename: '/my-file.jpg', field: 'callback-uid', - callbackId: 'test-callback-id', + callbackId, } const callbacks = mockCallbacks() handlePluginMessage(data, uid, callbacks) diff --git a/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.test.ts b/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.test.ts new file mode 100644 index 00000000..414136e5 --- /dev/null +++ b/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.test.ts @@ -0,0 +1,106 @@ +import { + ContextRequestMessage, + isContextRequestMessage, +} from './ContextRequestMessage' + +const stub: ContextRequestMessage = { + uid: 'abc', + story: { content: {} }, + action: 'get-context', + callbackId: 'test-callback-id', +} + +describe('ContextRequestMessage', () => { + it('should validate', () => { + expect(isContextRequestMessage(stub)).toEqual(true) + }) + + describe('The "action" property', () => { + it('equals "get-context"', () => { + expect( + isContextRequestMessage({ + ...stub, + action: 'anotherString', + }), + ).toEqual(false) + }) + }) + + describe('the "uid" property', () => { + it('is a string', () => { + expect( + isContextRequestMessage({ + ...stub, + uid: 'anything', + }), + ).toEqual(true) + }) + + it('is not undefined', () => { + expect( + isContextRequestMessage({ + ...stub, + uid: undefined, + }), + ).toEqual(false) + }) + + it('is not null', () => { + expect( + isContextRequestMessage({ + ...stub, + uid: null, + }), + ).toEqual(false) + }) + + it('is not a number', () => { + expect( + isContextRequestMessage({ + ...stub, + uid: 123, + }), + ).toEqual(false) + }) + }) + + describe('the "story" property', () => { + it('is not undefined', () => { + expect( + isContextRequestMessage({ + ...stub, + story: undefined, + }), + ).toEqual(false) + }) + + it('is not null', () => { + expect( + isContextRequestMessage({ + ...stub, + story: null, + }), + ).toEqual(false) + }) + }) + + describe('the "callbackId" property', () => { + it('is not undefined', () => { + expect( + isContextRequestMessage({ + ...stub, + callbackId: undefined, + }), + ).toEqual(false) + }) + + it('is not null', () => { + expect( + isContextRequestMessage({ + ...stub, + callbackId: null, + }), + ).toEqual(false) + }) + }) +}) diff --git a/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.ts b/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.ts index 2d746fb7..cdea79f3 100644 --- a/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.ts +++ b/packages/field-plugin/src/messaging/pluginMessage/containerToPluginMessage/ContextRequestMessage.ts @@ -5,6 +5,13 @@ import { isStoryData, StoryData } from './StoryData' //TODO: tests export type ContextRequestMessage = MessageToPlugin<'get-context'> & { story: StoryData + callbackId: string +} + +const hasCallbackId = ( + obj: unknown, +): obj is Pick => { + return hasKey(obj, 'callbackId') && typeof obj.callbackId === 'string' } export const isContextRequestMessage = ( @@ -13,4 +20,5 @@ export const isContextRequestMessage = ( isMessageToPlugin(data) && data.action === 'get-context' && hasKey(data, 'story') && + hasCallbackId(data) && isStoryData(data.story) diff --git a/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.test.ts b/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.test.ts index e91765c5..cac7cee6 100644 --- a/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.test.ts +++ b/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.test.ts @@ -6,10 +6,13 @@ import { } from './GetContextMessage' const uid = '-preview-abc-123' +const callbackId = 'test-callback-id' + const stub: GetContextMessage = { action: 'plugin-changed', event: 'getContext', uid, + callbackId, } describe('ValueChangeMessage', () => { @@ -34,7 +37,14 @@ describe('ValueChangeMessage', () => { }) describe('constructor', () => { it('includes the uid', () => { - expect(getContextMessage(uid)).toHaveProperty('uid', uid) + expect(getContextMessage(uid, callbackId)).toHaveProperty('uid', uid) + }) + + it('includes the callbackId', () => { + expect(getContextMessage(uid, callbackId)).toHaveProperty( + 'callbackId', + callbackId, + ) }) }) }) diff --git a/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.ts b/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.ts index 8ecd512e..8134171d 100644 --- a/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.ts +++ b/packages/field-plugin/src/messaging/pluginMessage/pluginToContainerMessage/GetContextMessage.ts @@ -1,12 +1,25 @@ +import { hasKey } from '../../../../src/utils' import { isMessageToContainer, MessageToContainer } from './MessageToContainer' -export type GetContextMessage = MessageToContainer<'getContext'> +export type GetContextMessage = MessageToContainer<'getContext'> & { + callbackId: string +} export const isGetContextMessage = (obj: unknown): obj is GetContextMessage => - isMessageToContainer(obj) && obj.event === 'getContext' + isMessageToContainer(obj) && obj.event === 'getContext' && hasCallbackId(obj) -export const getContextMessage = (uid: string): GetContextMessage => ({ +const hasCallbackId = ( + obj: unknown, +): obj is Pick => { + return hasKey(obj, 'callbackId') && typeof obj.callbackId === 'string' +} + +export const getContextMessage = ( + uid: string, + callbackId: string, +): GetContextMessage => ({ action: 'plugin-changed', event: 'getContext', uid, + callbackId, })