idosal/mcp-ui

Create a `ui-request-render-data`

Closed this issue · 23 comments

kentcdodds commented

Right now, ui-lifecycle-iframe-render-data is supposed to be posted from the parent when ui-lifecycle-iframe-ready is from the child. This kind of makes sense, but it's a bit of a surprising side-effect. And in my experience it's not working (hence my utility keeps a queue of render data messages so I can get them once my components are ready for them).

I think it would make more sense for me to specifically request the render data from the parent when I'm ready for it. That way I could even request it later if I wanted to.

Then I could use my existing sendMcpMessage utility: #100 (comment)

const renderData = await sendMcpMessage(
	'ui-request-render-data',
	{},
	{ schema: renderDataSchema },
)

Or something like that.

ibuildthecloud commented

@kentcdodds I'm kind of confused. Isn't the ui-lifecycle-frame-ready event the mechanism by which the iframe would request the data? It make sense it would be a one time thing. Is this just a situation where the mcp-ui client is just not behaving correctly?

aharvard commented

I could be wrong here, but there might be a race condition. I was able to confirm (in Goose) that ui-lifecycle-iframe-ready does get ui-lifecycle-iframe-render-data by wrapping my ui-lifecycle-iframe-ready in a set timeout.

kentcdodds commented

I'll have to do a little bit more testing and experimentation, but the point still stands that the ui-lifecycle-iframe-ready event does not seem to communicate effectively that it expects a response of the render data.

I really prefer the request-response style of tool calls, etc.

aharvard commented

In Goose we send a theme value (light or dark) as part of this data. MCP-UIs would benefit by automatically receiving this, right?

Do we (or did we have at some point in the recent past) an "initial" data for the UI lifecycle?

kentcdodds commented

You want to make sure that you don't conflict with whatever the tool is sending as part of the initial render data. I have a couple of ideas, but I'm curious what @idosal and @liady think.

liady commented

Just to align on the terms:

Render Data

  • A host can pass render data to an iframe (initially or at any point throughout its lifecycle).
    • If render data is present, the SDK posts it to the iframe on iframe load, using an ui-lifecycle-iframe-render-data message
    • The iframe can post ui-lifecycle-iframe-ready and get the same ui-lifecycle-iframe-render-data message back
    • The host can post ui-lifecycle-iframe-render-data as many times as needed (for example if the user toggles the theme and this theme needs to be sent to the iframe).
    • A waitForRenderData query param can be set on the iframe's URL to signal the iframe to wait before rendering, to prevent a flash in case the render data affects the appearance (for example custom css sent from the host). This is the reason that the render data is being sent as the response to the ui-lifecycle-iframe-ready event as well.

Requesting Data

  • An iframe can already (today) post a ui-request-data message, which includes a messageId field, requesting any arbitrary data from the host.
  • The host can respond with a ui-message-response with the same messageId, providing this data.
  • (This is true, by the way, to any message or action the iframe posts. If it includes a messageId - the host response will be send back to it using the same id.)

What's missing is a way for the iframe to request the render data again. Currently it's being sent after the iframe posts a ui-lifecycle-iframe-ready event, but as @kentcdodds points out - this can feel like a side effect, and is not a semantic way for requesting this data again, detached from the ready lifecycle event.

@kentcdodds propses here two things:

  1. Adding a dedicated ui-request-render-data message which will ask the host to send back the render data. It's still TBD whether it should be sent back using the existing ui-lifecycle-iframe-render-data message, or as a generic ui-message-response (like for the generic ui-request-data). I think it should be the former - it should respond with the ui-lifecycle-iframe-render-data again, for consistency.
  2. Adding a way for the iframe to specify a data schema when asking for data. I think this is a good idea that should also be applied to the generic ui-request-data message, and we should consider adding it.

I think we need to decide:

  1. Wheter we want a spearate ui-request-render-data message or can we add a key to the ui-request-data that we already have.
  2. If we're using the existing ui-request-data type then it should use the same mechanism we have to day for requesting data (responding with ui-message-response).
  3. If we are using a special ui-request-render-data message type - what should be the message type in the response. Again - I think that if we're using a special request type it should respond with the existing ui-lifecycle-iframe-render-data type.
kentcdodds commented

As far as I'm concerned, here's what I think we should do:

  1. I think it makes sense for render data to be separate from other types of data you can request
  2. I'm fine if ui-request-render-data uses the same mechanism as everything else (ui-message-response)
  3. I'm fine with ui-message-response

Honestly, it's not a huge deal to me either way. What's most important is that I get the render data after I am ready to receive it. Right now the postMessage shows up before I finish rendering so I miss it unless I set up my message handler ASAP (before I even render) which results in wonkiness like this:

// Module-level queue for render data events
const renderDataQueue: Array<{ type: string; payload: any }> = []

// Set up global listener immediately when module loads (only in the client)
if (typeof document !== 'undefined') {
	window.addEventListener(
		'message',
		(event) => {
			if (event.data?.type === 'ui-lifecycle-iframe-render-data') {
				renderDataQueue.push(event.data)
			}
		},
		{ once: false },
	)
}


export function waitForRenderData<RenderData>(
	schema: z.ZodSchema<RenderData>,
): Promise<RenderData> {
	return new Promise((resolve, reject) => {
		// Check if we already received the data
		const queuedEvent = renderDataQueue.find(
			(event) => event.type === 'ui-lifecycle-iframe-render-data',
		)
		if (queuedEvent) {
			const result = schema.safeParse(queuedEvent.payload.renderData)
			if (!result.success) {
				console.error('Invalid render data', queuedEvent.payload.renderData)
			}
			return result.success ? resolve(result.data) : reject(result.error)
		}

		// Otherwise, set up the normal listening logic
		function cleanup() {
			window.removeEventListener('message', handleMessage)
		}

		function handleMessage(event: MessageEvent) {
			if (event.data?.type !== 'ui-lifecycle-iframe-render-data') return

			const result = schema.safeParse(event.data.payload)
			cleanup()
			if (!result.success) {
				console.error('Invalid render data', event.data.payload)
			}
			return result.success ? resolve(result.data) : reject(result.error)
		}

		window.addEventListener('message', handleMessage, { once: true })
	})
}

When what I really should only have to do is this:

export function waitForRenderData<RenderData>(
	schema: z.ZodSchema<RenderData>,
): Promise<RenderData> {
	return new Promise((resolve, reject) => {
		function cleanup() {
			window.removeEventListener('message', handleMessage)
		}

		function handleMessage(event: MessageEvent) {
			if (event.data?.type !== 'ui-lifecycle-iframe-render-data') return

			const result = schema.safeParse(event.data.payload)
			cleanup()
			if (!result.success) {
				console.error('Invalid render data', event.data.payload)
			}
			return result.success ? resolve(result.data) : reject(result.error)
		}

		window.addEventListener('message', handleMessage, { once: true })
	})
}
liady commented

I think the best path forward here is:

  • To have a new dedicated ui-request-render-data event that can be fired from the iframe (optionally with a messageId), resulting in a standard ui-lifecycle-iframe-render-data event returned from the host (with that same messageId, if provided).

  • I think we'll leave the schema part to a later fix (we'd probably want it in other messages as well)

@kentcdodds what would you expect to return if no render data was provided by the host? null/empty object/no response message at all?

idosal commented

I agree that we shouldn't rely on ready, as it feels like a side effect. ui-request-render-data as a dedicated type makes sense as it's a first-class configuration property (although we will probably have more of these in the future).

This thread also leads me to believe we should completely decouple ready from the data (as a future breaking change). It'll help us avoid a situation where there are multiple ways to do the same thing, especially when it's implicit.

kentcdodds commented

expect to return if no render data was provided by the host?

I would expect undefined. If I get no message then I could be waiting with an event listener without knowing there's no point. If I get null, I would expect that means they provided data, but there was nothing to provide. If I get undefined that means they did not provide data.

liady commented

Just as a clarification: the idea behind sending the initial render data on ready is the use case of an iframe getting custom css from the outside (like Shopify mcp-ui supports). This can't be passed on the frame query params.
We want to avoid a style flash, so we need to iframe to wait for this data, and on the same time the host needs to know it's ready to receive this data before sending it.
This is all a workaround to solve the issue where this data has to be sent to frame asynchronously. (if we could have passed it on the query params this all would have been solved). There's a W3C proposal to allow for this synchronous data passing in the future.
So the logic behind tying "ready" and "render data" was around the initial use case. And it was meant explicitly to decouple the iframe sending its ready and setting the data event listener.

Now that we see more use cases emerging around this lifecycle it makes sense to add the explicit ui-request-render-data message type.

I'm not 100% sure that we don't need to send it also on ready (do we expect the iframes to always send two events - iframe-ready and request-render-data? Perhaps). But this breaking change can indeed be left for the next major version.

  • 👍1
kentcdodds commented

It doesn't bother me is there's an extra event emitted earlier. I do think there's a bug because the render data is being sent to me before my ui is ready. I'd that weren't the case I probably never would have brought this up

kentcdodds commented

So to be clear, this is how it should work?

export function waitForRenderData<RenderData>(
	schema: z.ZodSchema<RenderData>,
): Promise<RenderData> {
	return new Promise((resolve, reject) => {
		const messageId = crypto.randomUUID()

		window.parent.postMessage(
			{ type: 'ui-request-render-data', messageId },
			'*',
		)

		function handleMessage(event: MessageEvent) {
			if (event.data?.type !== 'ui-message-response') return
			if (event.data.messageId !== messageId) return
			window.removeEventListener('message', handleMessage)

			const { response, error } = event.data.payload

			if (error) return reject(error)
			if (!schema) return resolve(response)

			const parseResult = schema.safeParse(response)
			if (!parseResult.success) return reject(parseResult.error)

			return resolve(parseResult.data)
		}

		window.addEventListener('message', handleMessage)
	})
}

So if I record videos and teach students this, that will eventually be how it works?

liady commented

@kentcdodds I was thinking the response will use the same ui-lifecycle-iframe-render-data mechanism, for consistency.
See the example here (just replace the ui-lifecycle-iframe-ready type with ui-request-render-data).

So the return object will be:

{
  type: 'ui-lifecycle-iframe-render-data',
  payload: {
    renderData?: <the render data>, // undefined | unknown,
    error?: <any error, if occurred>, // undefined | unknown
  }
}

Your code would then be (changed message type and renderData payload key name):

export function waitForRenderData<RenderData>(
	schema: z.ZodSchema<RenderData>,
): Promise<RenderData> {
	return new Promise((resolve, reject) => {
		const messageId = crypto.randomUUID()

		window.parent.postMessage(
			{ type: 'ui-request-render-data', messageId },
			'*',
		)

		function handleMessage(event: MessageEvent) {
			if (event.data?.type !== 'ui-lifecycle-iframe-render-data') return // * CHANGED
			if (event.data.messageId !== messageId) return
			window.removeEventListener('message', handleMessage)

			const { renderData, error } = event.data.payload // * CHANGED

			if (error) return reject(error)
			if (!schema) return resolve(renderData)

			const parseResult = schema.safeParse(renderData)
			if (!parseResult.success) return reject(parseResult.error)

			return resolve(parseResult.data)
		}

		window.addEventListener('message', handleMessage)
	})
}

What do you think?

  • 👍1
kentcdodds commented

Looks good to me 👍

kentcdodds commented

Any updates on this? I'm going to be teaching MCP-UI next week (recording videos today/tomorrow) and I'm currently planning on teaching my workaround.

idosal commented

Any updates on this? I'm going to be teaching MCP-UI next week (recording videos today/tomorrow) and I'm currently planning on teaching my workaround.

On it

github-actions commented

🎉 This issue has been resolved in version 5.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

liady commented

Fixed by #111. Thanks @idosal for hopping on this!

  • 🎉1
github-actions commented

🎉 This issue has been resolved in version 1.0.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions commented

🎉 This issue has been resolved in version 1.0.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions commented

🎉 This issue has been resolved in version 5.12.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions commented

🎉 This issue has been resolved in version 5.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀