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: new simple framework agnostic api #158

Closed
wants to merge 1 commit into from
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
5 changes: 5 additions & 0 deletions examples/framesjs-starter/app/examples/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ export default function ExamplesIndexPage() {
Multi protocol
</Link>
</li>
<li>
<Link className="underline" href="/examples/simplified-api">
Simplified api
</Link>
</li>
<li>
<Link className="underline" href="/examples/slow-request">
Slow request
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { AnyJson } from "../new-api";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidfurlong @stephancill the issue here is that next.js fails to compile the route because it somehow doesn't allow to use react in route handlers. Maybe there is a way to work around this but I couldn't find one quickly.

import {
FrameImage,
Frame,
FramesApp,
FrameLink,
renderFramesToResponse,
} from "../react-api";

type FrameState = {
clicked: number;
};

function isFrameState(
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit of a strange function to have the user implement in the starter.

Copy link
Contributor

Choose a reason for hiding this comment

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

if user needs type safety they should define the type or zod validator and the lib should assert the correct types rather than have the user implement a type validation function like this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep those are possible solutions. Zod validator is the best I think as it actually validates.

frameState: AnyJson | undefined
): frameState is FrameState {
if (frameState && typeof frameState === "object" && "clcked" in frameState) {
return typeof frameState.clicked === "number";
}

return false;
}

export const framesApp = (
<FramesApp>
<Frame index>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think path="/" here is okay. It's not immediately clear what the index prop is doing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense but also index is used to detect initial frame. So perhaps initial prop would be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think path="/" is clearer. there could be multiple frame starting points

Copy link
Collaborator Author

@michalkvasnicak michalkvasnicak Mar 12, 2024

Choose a reason for hiding this comment

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

How would you set up multiple starting points @davidfurlong ? When you build frames app you have just one GET handler and only one Initial frame right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Where do you execute side effects like updating a db or executing a transaction?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That one is tricky and currently it is not implemented and I think that's the drawback of using React API as it doesn't allow us to use async render handlers.

Either we would have some sort of "hacky" solution I mean you could have on Frame a prop handler: () => Promise<void> or something like that.

Otherwise on agnostic API it would be just a handler property where you could define your own async function to do something when Frame is rendering.

{({ frameState }) => (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidfurlong @stephancill if you need to access the frame state (state passed from button click!, not the global frames state parsed from meta tag) you can use a function to render the frame.

Copy link
Contributor

Choose a reason for hiding this comment

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

not a big fan of render props - think they're awkward and many developers don't use them/don't know how to. It's hard to reason about whats injected and where it's coming from

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidfurlong who is our target audience? I'm confused because render props is common pattern as well. But I would dive that much into React as it is unnecessary to build frames app and makes the app and whole build bigger.

<>
<FrameImage>
<div>Test</div>
</FrameImage>
<FrameLink
path="/next"
label="Next"
state={{
clicked: isFrameState(frameState) ? frameState.clicked + 1 : 1,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidfurlong @stephancill this is a next frame state (the frame state of the destination frame will be set to this value)

}}
></FrameLink>
</>
)}
</Frame>
<Frame path="/next">
Copy link
Contributor

Choose a reason for hiding this comment

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

not a fan of putting many frames in one return, would rather have these be different routes in some way

{({ frameState }) => (
<>
<FrameImage>
<div>
Click #
{isFrameState(frameState)
? frameState.clicked
: "Not clicked yet"}
</div>
</FrameImage>
<FrameLink path="/" label="Back"></FrameLink>
</>
)}
</Frame>
</FramesApp>
);

/**
* GET renders always only initial frames
*/
export async function GET(req: Request) {
return renderFramesToResponse(framesApp, req);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidfurlong @stephancill this returns Response from Web API so is compatible with Remix and Next.js, by using some wrapper it could be also compatible with express, etc...

}

/**
* POST always render subsequent frames in reaction to buttons
*/
export async function POST(req: Request) {
return renderFramesToResponse(framesApp, req);
}
286 changes: 286 additions & 0 deletions examples/framesjs-starter/app/examples/simplified-api/new-api.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,286 @@
import assert from "assert";
Copy link
Collaborator Author

@michalkvasnicak michalkvasnicak Mar 9, 2024

Choose a reason for hiding this comment

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

This file contains framework agnostic frames building, you can use this one insteead of React. React is just a wrapper around it. So perhaps the nextjs route could be fixed by removing react API and use this one directly.

This API is not complete, I wanted to open the discussion.

@davidfurlong @stephancill

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason we are sticking to the react-like syntax? Feels like it is forcing us to keep all the logic contained within a single component

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

React just because whole frames were mainly promoted as nextjs thingy right? So I guess it is nice to also provide React syntax although in my opinion it unnecessarily limits us in order to provide the easiest API possible.

Also I think you meant to comment on react api right? Because this file doesn't contain React API, this is agnostic API using simple classes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also another question, how would you like to built frames apps? I think it's easier to think about some non abstract examples.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about @davidfurlong, but I'm not attached to React for this use case as it introduces a lot of magic abstractions.

As for how I would like to build frames apps, I like the <FrameContainer> pattern that frames.js uses today. Getting rid of the reducer would be a big improvement and button targets make it possible to have separate file-based routes (ideally without magic redirects). If a frame route can be simplified to something like

export async function POST(req: NextRequest) {
  const {frameMessage, state} = await getFrameMessage(req)
  
  // side effects

  return <FrameContainer>
    ...
  </FrameContainer>
  
}

then we would already be in a great spot. I know @davidfurlong is a lot more opinionated about this so maybe he has a different idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree with stephan on this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok so essentially just keep the API as is but change how post button works to use paths so you can have multiple handlers.

I still think that having some sort of router is closer to react developers than having multiple route files. But maybe that could be abstracted afterwards.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would rename the getFrameMessage as it returns message and state.

import { ImageResponse } from "@vercel/og";
import { ImageResponseOptions } from "next/server";
import { getFrameMessage } from "frames.js/next/server";
import { FrameActionDataParsedAndHubContext } from "frames.js";

/** A subset of JS objects that are serializable */
export type AnyJson = boolean | number | string | null | JsonArray | JsonMap;
export interface JsonMap {
[key: string]: AnyJson;
}
interface JsonArray extends Array<AnyJson> {}

interface IFrameImage {
renderToMetaTag(): Promise<string>;
}

export class Frame {
private image: IFrameImage | undefined;
private buttons: IFrameButton[] = [];
/**
* This is frame state coming from Button no the global app state comming from FramesApp
*/
private state: AnyJson | undefined = undefined;

getState() {
return this.state;
}

setState(state: AnyJson | undefined) {
this.state = state;
}

registerImage(
definitionOrURL: React.ReactElement | string | URL
): IFrameImage {
if (
typeof definitionOrURL === "object" &&
!(definitionOrURL instanceof URL)
) {
return (this.image = new FrameImage(definitionOrURL));
}

return (this.image = new FrameImageURL(new URL(definitionOrURL)));
}

registerPostButton(path: `/${string}`, label: string): FramePostButton {
if (this.buttons.length >= 4) {
throw new Error("Maximum number of buttons is 4");
}

const button = new FramePostButton(
path,
(this.buttons.length + 1) as AllowedButtonIndexes,
label
);

this.buttons.push(button);

return button;
}

registerExternalLinkButton(href: URL, label: string): FrameExternalButton {
if (this.buttons.length >= 4) {
throw new Error("Maximum number of buttons is 4");
}

const button = new FrameExternalButton(
href,
(this.buttons.length + 1) as AllowedButtonIndexes,
label
);

this.buttons.push(button);

return button;
}

async toHTML(app: FramesApp): Promise<string> {
assert(this.image, "Frame image is not set");

return `
<!DOCTYPE html>
<html>
<head>
<meta name="fc:frame" content="vNext" />
${await this.image.renderToMetaTag()}
<meta name="fc:frame:state" content="${JSON.stringify(app.getState())}" />
</head>
<body></body>
</html>
`.trim();
}
}

type AllowedButtonIndexes = 1 | 2 | 3 | 4;

interface IFrameButton {
renderToMetaTag(): string;
}

class FrameExternalButton implements IFrameButton {
private href: URL | undefined;
private index: AllowedButtonIndexes | undefined;
private label: string | undefined;

constructor(href: URL, index: AllowedButtonIndexes, label: string) {
this.href = href;
this.index = index;
this.label = label;
}
Comment on lines +102 to +111
Copy link
Contributor

Choose a reason for hiding this comment

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

Should try to stick more closely to the spec and avoid introducing new abstractions like href - buttons have an action and target

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep that makes sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok when I'm thinking about it, it doesn't matter whether it is href or action, the point is, that it must be an URL: I don't like having disjoint unions to define stuff that much, it is more clear to have specific classes or functions to create specific buttons (e.g. createMintButton, createPostButton or something like this than createButton({ action: 'post', ... })).


renderToMetaTag() {
assert(this.href, "Button href is not set");
assert(this.index, "Button index is not set");
assert(this.label, "Button label is not set");

return `
<meta name="fc:frame:button:${this.index}" content="${this.label}" />
<meta name="fc:frame:button:${this.index}:action" content="link" />
<meta name="fc:frame:button:${this.index}:target" content="${this.href.toString()}" />
`.trim();
}
}

class FramePostButton implements IFrameButton {
private path: `/${string}` | undefined;
private index: AllowedButtonIndexes | undefined;
private label: string | undefined;

constructor(path: `/${string}`, index: AllowedButtonIndexes, label: string) {
this.path = path;
this.index = index;
this.label = label;
}

renderToMetaTag(): string {
assert(this.path, "Button path is not set");
assert(this.index, "Button index is not set");
assert(this.label, "Button label is not set");

return `
Copy link
Contributor

Choose a reason for hiding this comment

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

these new lines are awkward - same with tabs

<meta name="fc:frame:button:${this.index}" content="${this.label}" />
<meta name="fc:frame:button:${this.index}:action" content="post" />
<meta name="fc:frame:button:${this.index}:target" content="${this.path}" />
`.trim();
}
}

export class FrameImageURL implements IFrameImage {
private url: URL;

constructor(url: URL) {
this.url = url;
}

async renderToMetaTag(): Promise<string> {
return `
<meta name="fc:frame:image" content="data:image/png;base64,${this.url.toString()}" />
<meta property="og:image" content="data:image/png;base64,${this.url.toString()}" />
`.trim();
}
}

export class FrameImage implements IFrameImage {
private definition: React.ReactElement;
private options: ImageResponseOptions | undefined;

constructor(definition: React.ReactElement, options?: ImageResponseOptions) {
this.definition = definition;
this.options = options;
}

async renderToMetaTag(): Promise<string> {
const imageResponse = new ImageResponse(
(
<div
style={{
display: "flex", // Use flex layout
flexDirection: "row", // Align items horizontally
alignItems: "stretch", // Stretch items to fill the container height
width: "100%",
height: "100vh", // Full viewport height
backgroundColor: "white",
}}
>
<div
style={{
display: "flex",
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this styling coming from

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

from FrameImage component

flexDirection: "column",
justifyContent: "center",
alignItems: "center",
lineHeight: 1.2,
fontSize: 36,
color: "black",
flex: 1,
overflow: "hidden",
}}
>
{this.definition}
</div>
</div>
),
this.options
);
const base64 = Buffer.from(await imageResponse.arrayBuffer()).toString(
"base64"
);

return `
<meta name="fc:frame:image" content="data:image/png;base64,${base64}" />
<meta property="og:image" content="data:image/png;base64,${base64}" />
`.trim();
}
}

export class FramesApp {
private state: JsonMap = {};
private frames: Record<`/${string}`, Frame> = {};
private frameMessage: FrameActionDataParsedAndHubContext | null = null;

setInitialState(state: JsonMap) {
this.state = state;
return this;
}

getState() {
return this.state;
}

getFrameMessage() {
return this.frameMessage;
}

registerFrame(path: `/${string}`) {
if (this.frames[path]) {
throw new Error(`Frame with path ${path} already exists`);
}

return (this.frames[path] = new Frame());
}

async renderToResponse(req: Request) {
const path = new URL(req.url).pathname as `/${string}`;

// @TODO if the request method is POST try to decode the frame message if any
// and set the state to detected state from message

// this is just naive implementation to show the idea
if (!this.frames[path]) {
return new Response("Not Found", { status: 404 });
}

// extract frame message if POST request with JSON body
if (req.method === "POST") {
try {
// set as property, this is cleared after request
this.frameMessage = await getFrameMessage(await req.json());
} catch (e) {
return new Response("Internal Server Error", { status: 500 });
}
}

try {
const frame = this.frames[path]!;

try {
// extract frameState from searchParams and pass it to frame
const frameState = new URL(req.url).searchParams.get("_fs");
frame.setState(frameState ? JSON.parse(frameState) : undefined);
} catch {}

const html = await frame.toHTML(this);

return new Response(html, {
headers: {
"Content-Type": "text/html",
},
});
} catch (e) {
return new Response("Internal Server Error", { status: 500 });
Copy link
Contributor

Choose a reason for hiding this comment

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

need meaningful error messages

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep this is just draft

} finally {
this.frameMessage = null;
}
}
}
Loading