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

Chore/docs and example fonts #232

Closed
wants to merge 2 commits into from
Closed

Conversation

davidfurlong
Copy link
Contributor

Change Summary

  • updates button docs
  • adds font example

Copy link

vercel bot commented Mar 24, 2024

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 25, 2024 9:30pm
framesjs-debugger ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2024 9:30pm

@davidfurlong davidfurlong changed the base branch from main to dev March 24, 2024 15:44
Comment on lines +57 to +58
You should make sure to clone any request you use, in order to not break other middleware. For example you can only call `const body = await ctx.request.clone().json();` once on a request.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we not just pass a clone of the request to each middleware?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was thinking the same - not sure what the overhead is or if there's any downside

Comment on lines 4 to 9
// without this line, this type of importing fonts doesn't work for some reason
export const runtime = "edge";

const regularFont = fetch(
new URL("/public/Inter-Regular.ttf", import.meta.url)
).then((res) => res.arrayBuffer());

Choose a reason for hiding this comment

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

It took me several attempts to figure this out, but for the nodejs runtime deployed to Vercel, it seems like the most reliable way was to use a combination of process.cwd() and fs functions:

https://github.com/chainpatrol/phishframe/blob/0fba55e76536ad57e12815c3b90ff0723124bc0a/src/app/frames/route.tsx#L147-L152

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dang thats tricky - thanks for following up. Will incorporate

Comment on lines 32 to 39
name: "Inter",
data: regularFontData,
weight: 400,
},
{
name: "Inter",
data: boldFontData,
weight: 700,
Copy link
Contributor

Choose a reason for hiding this comment

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

typos: should be regularFont and boldFont

Copy link
Contributor

Choose a reason for hiding this comment

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

they also need to be awaited

Copy link
Contributor Author

Choose a reason for hiding this comment

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

think you're missing line 16

Copy link
Contributor

Choose a reason for hiding this comment

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

oops missed that sorry

@stephancill
Copy link
Contributor

@davidfurlong i have added middleware docs in #240 and a more focused custom middleware example in #248 - these aspects can be removed from this pr if you agree

@stephancill stephancill mentioned this pull request Mar 27, 2024
3 tasks
@stephancill
Copy link
Contributor

Closing since changes from this PR have been included in #252 and #240

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants