feat(lib): handle callbackId in contextRequest

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],
   )

packages/container/src/dom/createContainerMessageListener.ts

@@ -55,9 +55,12 @@ export const createContainerMessageListener: CreateContainerListener = (
     } else if (isHeightChangeMessage(message)) {
       eventHandlers.setHeight(message.height)
     } else if (isAssetModalChangeMessage(message)) {
-      eventHandlers.selectAsset(message.callbackId, message.field ?? '')
+      eventHandlers.selectAsset(
+        message.callbackId as string,
+        message.field ?? '',
+      )
     } else if (isGetContextMessage(message)) {
-      eventHandlers.requestContext()
+      eventHandlers.requestContext(message.callbackId as string)
     } else {
       console.warn(
         `Container received unknown message from plugin: ${JSON.stringify(

packages/field-plugin/src/createFieldPlugin/FieldPluginActions.ts

@@ -1,9 +1,9 @@
-import { Asset } from '../messaging'
+import { Asset, ContextRequestMessage } from '../messaging'
 
 export type SetStateAction<T> = T | ((value: T) => T)
 export type SetContent = <C>(setContentAction: SetStateAction<C>) => void
 export type SetModalOpen = (setModalOpenAction: SetStateAction<boolean>) => void
-export type RequestContext = () => void
+export type RequestContext = () => Promise<ContextRequestMessage>
 export type SelectAsset = () => Promise<Asset>
 
 export type FieldPluginActions = {

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<AssetModalChangeMessage>),
       )
     })
-    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 },

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 `selectAsset` 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,

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)

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)
+    })
+  })
+})

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<ContextRequestMessage, 'callbackId'> => {
+  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)

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,
+      )
     })
   })
 })