Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ web_modules/

# PNPM
.pnpm-store/
.pnpm-home/

# TypeScript cache
*.tsbuildinfo
Expand Down
9 changes: 5 additions & 4 deletions pnpm-workspace.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
packages:
# all packages in subdirs of sdks/typescript/
- 'sdks/typescript/*'
- 'examples/*'
- 'docs'
- sdks/typescript/*
- examples/*
- docs
onlyBuiltDependencies:
- esbuild
47 changes: 31 additions & 16 deletions sdks/typescript/client/scripts/proxy/index.html
Copy link
Author

Choose a reason for hiding this comment

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

@idosal @liady this is the main change to support the double iframe exactly the same way as chatGTP.

also I think line 6 the meta CSP is not required, I had to remove it since the CSP can be set on the actual remote html and needs to be super relaxed otherwise nothing load

Copy link
Author

Choose a reason for hiding this comment

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

@idosal you recently added the double proxy. what client was that for? I don't think this is a breaking change since it is the code clients need to add to their CDN anyway but curious if you tested the previous implementation

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not used in production yet, so we can change it.

Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,42 @@
if (contentType === 'rawhtml') {
// Double-iframe raw HTML mode (HTML sent via postMessage)
const inner = document.createElement('iframe');
let pendingHtml = null;
const renderHtmlInIframe = (markup) => {
const doc = inner.contentDocument || inner.contentWindow?.document;
if (!doc) return false;
try {
doc.open();
doc.write(markup);
doc.close();
return true;
} catch (error) {
return false;
}
};
inner.addEventListener('load', () => {
if (pendingHtml !== null && renderHtmlInIframe(pendingHtml)) {
pendingHtml = null;
}
});
inner.style = 'width:100%; height:100%; border:none;';
// sandbox will be set from postMessage payload; default minimal before html arrives
inner.setAttribute('sandbox', 'allow-scripts');
// Default sandbox ensures same-origin access for DOM writing; parent may override
inner.setAttribute('sandbox', 'allow-scripts allow-same-origin');
inner.src = 'about:blank';
document.body.appendChild(inner);

// Wait for HTML content from parent
window.addEventListener('message', (event) => {
if (event.source === window.parent) {
if (event.data && event.data.type === 'ui-html-content') {
const payload = event.data.payload || {};
const html = payload.html;
const sandbox = payload.sandbox;
if (typeof sandbox === 'string') {
inner.setAttribute('sandbox', sandbox);
}
if (typeof html === 'string') {
inner.srcdoc = html;
Copy link
Author

Choose a reason for hiding this comment

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

the key was to use document.write() instead of srcdoc.

srcdoc changes the root url and breaks client side navigation.

inner.src: 'about:blank'

and document.write() do the trick

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great catch!

}
} else {
if (inner && inner.contentWindow) {
inner.contentWindow.postMessage(event.data, '*');
if (event.source === window.parent && event.data && event.data.type === 'ui-html-content') {
const payload = event.data.payload || {};
const html = payload.html;
const sandbox = payload.sandbox;
if (typeof sandbox === 'string') {
inner.setAttribute('sandbox', sandbox);
}
if (typeof html === 'string') {
if (!renderHtmlInIframe(html)) {
pendingHtml = html;
}
}
} else if (event.source === inner.contentWindow) {
Expand Down
25 changes: 20 additions & 5 deletions sdks/typescript/client/src/components/HTMLResourceRenderer.tsx
Copy link
Author

Choose a reason for hiding this comment

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

@liady @idosal made all the changes here and added a new condition to HTMLResourceRenderer since is practically the same but let me know if you'd prefer to create a new file SkybridgeResourceRenderer and put the rendering there instead

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export type HTMLResourceRendererProps = {
style?: React.CSSProperties;
proxy?: string;
iframeRenderData?: Record<string, unknown>;
toolInput?: Record<string, unknown>;
toolName?: string;
toolResponseMetadata?: Record<string, unknown>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@infoxicator WDYT about formalizing these fields to be in the render data?

Copy link
Author

Choose a reason for hiding this comment

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

@idosal I am leaning more towards a cleaner interface like here: so we can extend it and add more fields in the future if needed without having to pollute the props of the main renderUIResource component.

Copy link
Author

Choose a reason for hiding this comment

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

Been working on shipping the postman integration. But still giving this some thought. I like the new props jsut ensuring they align with mcp-ui and all the other resource types not just chatgpt.

For example. ToolResponse and widget state in the context of chatgpt is the initial data. but I think there might be a case where we would like to pass arbitrary data to a widget that are not those. Currently I am merging them. into a single object for chatgpt but considering all the implications of that.

Maybe is safer just to keep them separate?

autoResizeIframe?: boolean | { width?: boolean; height?: boolean };
sandboxPermissions?: string;
iframeProps?: Omit<React.HTMLAttributes<HTMLIFrameElement>, 'src' | 'srcDoc' | 'style'> & {
Expand Down Expand Up @@ -43,18 +46,16 @@ export const HTMLResourceRenderer = ({
style,
proxy,
iframeRenderData,
toolInput,
toolName,
toolResponseMetadata,
autoResizeIframe,
sandboxPermissions,
iframeProps,
}: HTMLResourceRendererProps) => {
const iframeRef = useRef<HTMLIFrameElement | null>(null);
useImperativeHandle(iframeProps?.ref, () => iframeRef.current as HTMLIFrameElement);

const { error, iframeSrc, iframeRenderMode, htmlString } = useMemo(
() => processHTMLResource(resource, proxy),
[resource, proxy],
);

const uiMetadata = useMemo(() => getUIResourceMetadata(resource), [resource]);
const preferredFrameSize = uiMetadata[UIMetadataKey.PREFERRED_FRAME_SIZE] ?? ['100%', '100%'];
const metadataInitialRenderData = uiMetadata[UIMetadataKey.INITIAL_RENDER_DATA] ?? undefined;
Expand All @@ -69,6 +70,20 @@ export const HTMLResourceRenderer = ({
};
}, [iframeRenderData, metadataInitialRenderData]);

const { error, iframeSrc, iframeRenderMode, htmlString } = useMemo(
() =>
processHTMLResource(
resource,
proxy,
initialRenderData,
toolInput,
toolName,
toolResponseMetadata
),
[resource, proxy]
);


const iframeSrcToRender = useMemo(() => {
if (iframeSrc && initialRenderData) {
const iframeUrl = new URL(iframeSrc);
Expand Down
4 changes: 4 additions & 0 deletions sdks/typescript/client/src/components/UIResourceRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ function getContentType(
if (resource.mimeType === 'text/html') {
return 'rawHtml';
}
if (resource.mimeType === 'text/html+skybridge') {
return 'skybridge';
}
if (resource.mimeType === 'text/uri-list') {
return 'externalUrl';
}
Expand All @@ -40,6 +43,7 @@ export const UIResourceRenderer = (props: UIResourceRendererProps) => {

switch (contentType) {
case 'rawHtml':
case 'skybridge':
case 'externalUrl': {
return <HTMLResourceRenderer resource={resource} onUIAction={onUIAction} {...htmlProps} />;
}
Expand Down
7 changes: 6 additions & 1 deletion sdks/typescript/client/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import React from 'react';

export type UIActionType = 'tool' | 'prompt' | 'link' | 'intent' | 'notify';

export const ALL_RESOURCE_CONTENT_TYPES = ['rawHtml', 'externalUrl', 'remoteDom'] as const;
export const ALL_RESOURCE_CONTENT_TYPES = [
'rawHtml',
'externalUrl',
'remoteDom',
'skybridge',
Copy link
Author

Choose a reason for hiding this comment

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

@idosal @liady any preference with this name? skybridge is what they call it

] as const;
export type ResourceContentType = (typeof ALL_RESOURCE_CONTENT_TYPES)[number];

type GenericActionMessage = {
Expand Down
Loading