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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Generate user profile image on demand when the url is shared #1091

Closed
1 of 2 tasks
bdougie opened this issue Apr 12, 2023 · 13 comments 路 Fixed by #1095
Closed
1 of 2 tasks

Feature: Generate user profile image on demand when the url is shared #1091

bdougie opened this issue Apr 12, 2023 · 13 comments 路 Fixed by #1095

Comments

@bdougie
Copy link
Member

bdougie commented Apr 12, 2023

Type of feature

馃崟 Feature

Current behavior

Screen Shot 2023-04-12 at 9 39 21 AM

Suggested solution

Open graph images should hit https://opengraph.opensauced.pizza/v1/users/bdougie and generate this image in a Tweet when a profile is shared.

https://graph.opensauced.xyz/users/bdougie.png

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct

Contributing Docs

  • I agree to follow this project's Contribution Docs
@Deadreyo
Copy link
Contributor

Deadreyo commented Apr 12, 2023

User pages need to be server-side rendered to allow for this to happen, but I remember asking about this and there were reasons why we didn't initially do that. Would love clarification on this and if it still stands.
cc @0-vortex @brandonroberts

@0-vortex
Copy link
Contributor

User pages need to be server-side rendered to allow for this to happen, but I remember asking about this and there were reasons why we didn't initially do that. Would love clarification on this and if it still stands.
cc @0-vortex @brandonroberts

Take the code from #1087

@brandonroberts
Copy link
Contributor

brandonroberts commented Apr 12, 2023

Why does the page need to be server rendered for this? I thought using the image was adding metadata to the head to display the OpenGraph image?

@Deadreyo
Copy link
Contributor

Deadreyo commented Apr 12, 2023

Why does the page need to be server rendered for this? I thought added the image was adding metadata to the head to display the OpenGraph image?

How would the open graph images and tags load or work if the link embedded is client-side rendered? Also given that the user page is a dynamic route

@0-vortex
Copy link
Contributor

you don't share the image, you share the page ... the page has a meta tag for the twitter image, so BOTH need a server for a user to magically get the embed.
this is all required for SEO so don't undertand the confusion; this ticket is done by doing just that ... if we don't want to do that then yeah we need to hit the graph backend to get the end result social image that would result before letting the user share - it's same amount of work doing it from ui to evade the seo work

@bdougie
Copy link
Member Author

bdougie commented Apr 13, 2023

I am more confused by that response @0-vortex

Here is link on social images from LogRocket.

My naive approach would be dynamically fetching the image from opengraph.opesauced.pizza and adding it here: https://github.com/open-sauced/insights/blob/479eb625c6496bd762c27a12b742dc7713a8e869/layouts/SEO/SEO.tsx#L19

Now doing that dynamically is up for interpretation, but consider this is a normal interaction on the internet. We should lean towards engineering the best experience for the user.

@brandonroberts
Copy link
Contributor

We're already using the next/head here in the user profile, so I would think we can just add the meta information for the OpenGraph image there. We only need the username, which is already provided through the URL path

https://github.com/open-sauced/insights/blob/beta/pages/user/%5Busername%5D/index.tsx#L44

@OgDev-01
Copy link
Contributor

@Deadreyo I think Ted already implemented a solution for this on #1087
Setting the image property for the SEO to https://opengraph-dev.opensauced.cloud/users/${username}.png should work for now. SSR can come into play later 馃憤

@Deadreyo
Copy link
Contributor

Deadreyo commented Apr 13, 2023

@brandonroberts @OgDev-01

I'm sorry but the dynamic nature of user profiles make it not possible to do this with Nextjs' default rendering behavior: static site generation. (Experiment attached below for clarification)

The problem

Next.js by default uses static site generation for pages unless stated otherwise. Meaning the pages are pre-rendered during build time and then served statically to the client. This means it gets all the SEO and meta tags and initial stuff during build time. Data loads with UseEffects and hooks later on but on the client-side not on the server-side, so when a page requests the meta tags for the requested page, it simply will only acknowledge the pre-rendered tags, which won't be related to the user at all.

This means that, unless we are going to build a page for every single user at build time, this approach simply won't display the dynamic meta tags when sharing the link.

Experiment to proved this:

Followed @brandonroberts suggested solution, simply adding a Head and a title inside:

<SEO
        title={`${contributorLogin} | OpenSauced`}
/>

This is equivalent to making a Head with title inside.

Then inspecting the very initial HTML returned (which will be used by sites for data), we see the title as: <title>undefined | OpenSauced</title>.

Explanation of this is that, since the page uses static site generation, at build time, the username was undefined and there was no user, so it was resolved to this and is now sent on every request.

It is true that after the initial HTML render these meta tags will update to the correct data, but this will be on the client-side after the initial render, which won't be noticed by the website displaying the link's info, basically making it useless.

Solution

To correctly have sharing the link display the correct meta tags and info, the meta tags must be dynamically generated and filled with the correct data from the server before being sent. and since we simply can't pre-render a user page for every single user we have, we will have to dynamically generate the page on demand, I.e. going from the default static site generation to server side generation.

Hope this clears any confusion about why I have to and already went with SSR for user pages!

@OgDev-01
Copy link
Contributor

So what is the current challenge with SSR on the user page?

@Deadreyo
Copy link
Contributor

So what is the current challenge with SSR on the user page?

Still working on it, but wanted to announce this change as IMO it's important to note it out. (since SSR is a heavier load on the server).

@github-actions
Copy link

馃帀 This issue has been resolved in version 1.42.0-beta.1 馃帀

The release is available on GitHub release

Your semantic-release bot 馃摝馃殌

@github-actions
Copy link

馃帀 This issue has been resolved in version 1.42.0 馃帀

The release is available on GitHub release

Your semantic-release bot 馃摝馃殌

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

Successfully merging a pull request may close this issue.

5 participants