-
Notifications
You must be signed in to change notification settings - Fork 809
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
Improve meta handling (NOT included => default Farcaster Frame) #811
Conversation
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.
Nicceee, Thanks @carletex !! Looks great and I think its nice to have!
Things which we can handle in maybe different PR :
About NextJS Metadata. layout.tsx and getMetadata.ts feel a bit duplicated. Is there a way around it? Feels weird to have to add the frame meta to both places.
Yup agree, also with vercel giving gated URL its PITA to that baseUrl
in both files
I like the way how vercel in their docs have created shared-metadata.ts
file, maybe we could have that for all the common things in metadata ?
VERCEL_URL: We use it for the base URL, and now Vercel returns the "protected deployment route" (instead of the production one. Related to #807 I think).. which make this a bit useless:
:( , Just created vercel/vercel#11438 and also asked question on discord
(Sorry for deviating from the original PR haha)
Seems like the obvious way to fix it. What am I missing here? Besides that, we could even add baseUrl to |
Niiiceee !!! I thought having
+1 for this, looking at Next's discord people have been using / suggesting to hardcode the domain URL directly and there's no way to get domain from vercel's env var So makes sense to have it in scaffold.config.ts Nitpick: Maybe we could use Tysm @carletex !! |
+, tried to find a better way to set baseUrl too, but no luck Thanks for pr and interesting discussion! |
Thanks all for the comments!! I'm gonna do some tinkering and propose an initial solution. Also trying this now: https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase |
The metadataBase doesn't seem to work for me: https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase This would be nice too.... but we have the VERCEL_URL issue, where it points to a usually protected deployment route (instead of the PROD URL) If we don't make it work, maybe we just can leave it as it's in the PR (at least we are deduplicating the layout / getMetadata code) |
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.
The metadataBase doesn't seem to work for me: https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase
:( , But I really love now have we have centralised / one getMetadata
function ! Really easy to edit !!
Thanks Carlos !!!
So for now, decided to remove the Farcaster frame stuff from this PR. See this convo for context: #811 (comment) So at the end this PR just remove some duplication with the metadata. plus https://twitter.com/rauchg/status/1787664435097538776 This happened finally!! 🔥 🔥 |
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.
Love how metadata is handled now, and the new VERCEL_PROJECT_PRODUCTION_URL
!
Looking good to me 🔥 GJ @carletex
=============================
I think this behavior is OK, since vercel only saves 1 variable for the project, but just checking:
Testing the unfurl with the production link, realized only works if you share the main domain from the project, not the deployment URL.
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.
The metadataBase doesn't seem to work for me: https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase
This would be nice too.... but we have the VERCEL_URL issue, where it points to a usually protected deployment route (instead of the PROD URL)
If we don't make it work, maybe we just can leave it as it's in the PR (at least we are deduplicating the layout / getMetadata code)
Maybe this will solved in latest Next version not completely sure though ? https://x.com/WillemJaap_/status/1787738993964048560
realized only works if you share the main domain from the project, not the deployment URL.
Yes, actually this problem is kind of related to #807....the deployment URL are unique and only accessible to team / authenticated users and hence when you share that URL in whatsapp / any other app, that app won't be able access / fetch anything from that URL
If you try to open https://new-metadata-47koiot3h-tokodevs-projects.vercel.app in incognito it won't work there too.
Merging this thanks all !!
I was thinking about adding a default "dummy" Farcaster Frame for SE-2. I say "dummy" because you usually add a backend route to handle different actions within the frame (but that would be too much)
To test locally, you can use: https://github.com/coinbase/onchainkit
-> clone, go to the framegear folder, npm install, npm run dev... and you get this:
A few things:
layout.tsx
andgetMetadata.ts
feel a bit duplicated. Is there a way around it? Feels weird to have to add the frame meta to both places.Any ideas? (we can discuss in another issue/PR, sorry haha)