feat: new simple framework agnostic api #158

michalkvasnicak · 2024-03-09T18:23:20Z

Change Summary

This PR is just another attempt to simplify the frames app building and also provide framework agnostic API that can be used outside of Next.js

Merge Checklist

PR has a Changeset
PR includes documentation if necessary
PR updates the boilerplates if necessary

vercel · 2024-03-09T18:23:22Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
frames-js	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 9, 2024 6:26pm
framesjs-debugger	❌ Failed (Inspect)			Mar 9, 2024 6:26pm

michalkvasnicak · 2024-03-09T18:25:01Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

@@ -0,0 +1,72 @@
+import { AnyJson } from "../new-api";


@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.

michalkvasnicak · 2024-03-09T18:25:47Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+export const framesApp = (
+  <FramesApp>
+    <Frame index>
+      {({ frameState }) => (


@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.

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

@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.

michalkvasnicak · 2024-03-09T18:26:14Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+            path="/next"
+            label="Next"
+            state={{
+              clicked: isFrameState(frameState) ? frameState.clicked + 1 : 1,


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

michalkvasnicak · 2024-03-09T18:27:10Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

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


@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...

michalkvasnicak · 2024-03-09T18:28:16Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

@@ -0,0 +1,286 @@
+import assert from "assert";


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

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

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.

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

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.

agree with stephan on this

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.

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

michalkvasnicak · 2024-03-09T18:29:30Z

@davidfurlong @stephancill this one is a draft to open a discussion with you. This API is framework agnostic so it should be really easy to integrate frames anywhere and also provide more templates to #143 so we can support remix, express, etc.

stephancill · 2024-03-11T14:24:27Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+
+export const framesApp = (
+  <FramesApp>
+    <Frame index>


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

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

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

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?

stephancill · 2024-03-11T14:39:08Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

+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;
+  }


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

Yep that makes sense.

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', ... })).

stephancill · 2024-03-11T14:44:55Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+
+export const framesApp = (
+  <FramesApp>
+    <Frame index>


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

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.

stephancill · 2024-03-11T14:51:35Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

@@ -0,0 +1,286 @@
+import assert from "assert";


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

davidfurlong

hm I feel like this PR is rewriting a largely declarative (JSX) definition into some sort of nested imperative library for rendering it. Not a fan of the overall direction.

Feel it would be much cleaner if dev returned json (maybe via jsx syntax, maybe not?), then lib used a zod validator on that json to catch errors. Declarative > Imperative, Functional > Object oriented is my north star here. Feels like we're going the wrong direction on those fronts somewhat.

This PR is intended to solve

Decoupling the library from next.js
Replacing useReducer state handling with an alternative that is easier to reason about

I think we should discuss the APIs for that before diving into code. For example the frame as an object rather than functions per frame is a discussion we should have via pseudocode before writing this whole PR out - I don't think an object with all frames as render functions is the way to go

Lots of my PR comments are on granular details that may not matter at all if we don't go in this direction - we need to agree on the high level before we dive into writing/reviewing the low levels

davidfurlong · 2024-03-12T13:50:40Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+  clicked: number;
+};
+
+function isFrameState(


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

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

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

davidfurlong · 2024-03-12T13:53:08Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+
+export const framesApp = (
+  <FramesApp>
+    <Frame index>


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

davidfurlong · 2024-03-12T13:53:30Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+        </>
+      )}
+    </Frame>
+    <Frame path="/next">


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

davidfurlong · 2024-03-12T13:54:47Z

examples/framesjs-starter/app/examples/simplified-api/[[...routes]]/route.tsx

+export const framesApp = (
+  <FramesApp>
+    <Frame index>
+      {({ frameState }) => (


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

davidfurlong · 2024-03-12T13:56:09Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

@@ -0,0 +1,286 @@
+import assert from "assert";


agree with stephan on this

davidfurlong · 2024-03-12T13:58:57Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

+        >
+          <div
+            style={{
+              display: "flex",


where is this styling coming from

from FrameImage component

davidfurlong · 2024-03-12T14:02:16Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

+        },
+      });
+    } catch (e) {
+      return new Response("Internal Server Error", { status: 500 });


need meaningful error messages

yep this is just draft

davidfurlong · 2024-03-12T14:02:33Z

examples/framesjs-starter/app/examples/simplified-api/new-api.tsx

+    assert(this.index, "Button index is not set");
+    assert(this.label, "Button label is not set");
+
+    return `


these new lines are awkward - same with tabs

michalkvasnicak · 2024-03-12T16:30:12Z

hm I feel like this PR is rewriting a largely declarative (JSX) definition into some sort of nested imperative library for rendering it. Not a fan of the overall direction.

Feel it would be much cleaner if dev returned json (maybe via jsx syntax, maybe not?), then lib used a zod validator on that json to catch errors. Declarative > Imperative, Functional > Object oriented is my north star here. Feels like we're going the wrong direction on those fronts somewhat.

This PR is intended to solve

Decoupling the library from next.js

Replacing useReducer state handling with an alternative that is easier to reason about

I think we should discuss the APIs for that before diving into code. For example the frame as an object rather than functions per frame is a discussion we should have via pseudocode before writing this whole PR out - I don't think an object with all frames as render functions is the way to go

Lots of my PR comments are on granular details that may not matter at all if we don't go in this direction - we need to agree on the high level before we dive into writing/reviewing the low levels

@davidfurlong I agree with you on this. But each change that is happening is somehow always tied to "how it will be used in nextjs?".

In my opinion it would be great to move away from React if possible as that will allow you to have smaller builds (vercel, lambda, cloudflare). Only code that needs React in some way is @vercel/og although this can be replaced by object definition.

I agree that specifying frame as an object and validating it using zod is another way to go and probably nice as it is also possible to infer types from the schema. So perhaps that would be nice way to build frames app.

// route.ts -- initial frame, handled only by GET request
export async function GET(req) {
  // not a good name for function, something different would be better
   const app = frames(req, initialState);

   return app.frame({
      image: {}, // either React definition or JSX object notation e.g. { type: Component, props: {...}}
      buttons: [
         { action: 'post', label: 'Next', target: '/next-route', state: { clicks: 1 } }
      ]
   }); 
}

export async function POST(req) {
  const app = frames(req, initialState);

  return app.frame({
      image: {}, // either React definition or JSX object notation e.g. { type: Component, props: {...}}
      buttons: [
         { action: 'post', label: 'Next', target: '/next-route', state: { clicks: app.state.clicks + 1 } }
      ]
   }); // req necessary to correctly build the target for `post` button
}

// /next-route/route.ts -- next frame after button click, only POST request
export async function POST(req) {
   const app = frames(req, initialState);

  app.setState() // this could be used for app state `meta fc:frame:state` manipulation? As addition to button provided state

   return app.frame({
      image: {}, // either React definition or JSX object notation e.g. { type: Component, props: {...}}
      buttons: [
         // use app.state here as next state when going back, so we have button specific state
         { action: 'post', label: 'Back', target: '/', state: app.state }
      ]
   });
}

something like this? app.state is weird I know, perhaps app.buttonState would be clearer.

stephancill · 2024-03-14T08:01:57Z

@michalkvasnicak I think your example is on the right track and we should proceed in this direction

michalkvasnicak · 2024-03-14T08:25:11Z

@stephancill also frames() function should handle message parsing, state parsing and validation of the message right? We want to make the public API surface minimal so it is easy and fast to build apps.

stephancill · 2024-03-14T08:44:53Z

@michalkvasnicak that's right. could be configurable via an optional second argument similar to getFrameMessage

davidfurlong · 2024-03-14T08:59:13Z

Hold up I think theres a bunch of things that are off about this

stephancill · 2024-03-20T14:09:27Z

closing this in favour of #175

feat: new simple framework agnostic api

ad7cc01

michalkvasnicak self-assigned this Mar 9, 2024

vercel bot had a problem deploying to Preview – framesjs-debugger March 9, 2024 18:24 Failure

michalkvasnicak commented Mar 9, 2024

View reviewed changes

vercel bot deployed to Preview – frames-js March 9, 2024 18:26 View deployment

michalkvasnicak commented Mar 9, 2024

View reviewed changes

michalkvasnicak requested review from davidfurlong and stephancill March 9, 2024 18:28

stephancill reviewed Mar 11, 2024

View reviewed changes

stephancill mentioned this pull request Mar 11, 2024

feat: simplify building frames #92

Closed

4 tasks

davidfurlong requested changes Mar 12, 2024

View reviewed changes

Base automatically changed from dev to main March 18, 2024 15:38

stephancill closed this Mar 20, 2024

feat: new simple framework agnostic api #158

feat: new simple framework agnostic api #158

Conversation

michalkvasnicak commented Mar 9, 2024

Change Summary

Merge Checklist

vercel bot commented Mar 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalkvasnicak Mar 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalkvasnicak commented Mar 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalkvasnicak Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfurlong left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalkvasnicak commented Mar 12, 2024

stephancill commented Mar 14, 2024

michalkvasnicak commented Mar 14, 2024

stephancill commented Mar 14, 2024

davidfurlong commented Mar 14, 2024

stephancill commented Mar 20, 2024

vercel bot commented Mar 9, 2024 •

edited

Loading

michalkvasnicak Mar 9, 2024 •

edited

Loading

michalkvasnicak Mar 12, 2024 •

edited

Loading

davidfurlong left a comment •

edited

Loading