-
Notifications
You must be signed in to change notification settings - Fork 100
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
import { AnyJson } from "../new-api"; | ||
import { | ||
FrameImage, | ||
Frame, | ||
FramesApp, | ||
FrameLink, | ||
renderFramesToResponse, | ||
} from "../react-api"; | ||
|
||
type FrameState = { | ||
clicked: number; | ||
}; | ||
|
||
function isFrameState( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense but also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 }) => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @davidfurlong @stephancill this returns |
||
} | ||
|
||
/** | ||
* POST always render subsequent frames in reaction to buttons | ||
*/ | ||
export async function POST(req: Request) { | ||
return renderFramesToResponse(framesApp, req); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,286 @@ | ||
import assert from "assert"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree with stephan on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok so essentially just keep the API as is but change how 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rename the |
||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep that makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
||
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 ` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where is this styling coming from There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need meaningful error messages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep this is just draft |
||
} finally { | ||
this.frameMessage = null; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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.