Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lib): handle callbackId in contextRequest #261

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/container/src/components/FieldPluginContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -157,11 +158,12 @@ const useSandbox = (
)

const onContextRequested = useCallback(
() =>
(callbackId: string) =>
dispatchContextRequest({
uid,
action: 'get-context',
story: loadedData.story,
callbackId,
}),
[uid, dispatchContextRequest, loadedData.story],
)
Expand Down
5 changes: 2 additions & 3 deletions packages/container/src/dom/createContainerMessageListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
isPluginLoadedMessage,
isValueChangeMessage,
PluginLoadedMessage,
RequestContext,
SetContent,
SetModalOpen,
} from '@storyblok/field-plugin'
Expand All @@ -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
}

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createPluginActions } from './createPluginActions'
import {
AssetModalChangeMessage,
GetContextMessage,
HeightChangeMessage,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import was not being used

ModalChangeMessage,
ValueChangeMessage,
} from '../../messaging'
Expand Down Expand Up @@ -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({
Expand All @@ -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()
Expand All @@ -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 },
Expand All @@ -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 {
Expand All @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
Asset,
assetFromAssetSelectedMessage,
assetModalChangeMessage,
ContextRequestMessage,
getContextMessage,
heightChangeMessage,
modalChangeMessage,
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
} from '../../../messaging'

const uid = 'abc123'
const callbackId = 'test-callback-id'

const mockCallbacks = (): PluginMessageCallbacks => ({
onStateChange: jest.fn(),
onContextRequest: jest.fn(),
Expand Down Expand Up @@ -73,6 +75,7 @@ describe('handlePluginMessage', () => {
const data: ContextRequestMessage = {
action: 'get-context',
uid,
callbackId,
story: { content: {} },
}
const callbacks = mockCallbacks()
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand All @@ -13,4 +20,5 @@ export const isContextRequestMessage = (
isMessageToPlugin(data) &&
data.action === 'get-context' &&
hasKey(data, 'story') &&
hasCallbackId(data) &&
isStoryData(data.story)
Loading