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

Implement dynamic OpenGraph image url support for crate pages #10464

Merged
merged 10 commits into from
Feb 7, 2025

Conversation

hdoordt
Copy link
Contributor

@hdoordt hdoordt commented Jan 28, 2025

Related discussion: #9928

I replaced tower_http::services::ServeFile with a manually built response, and attempted to mimic its behavior. Furthermore, I replaced the hard coded open graph image url from app/index.html with a Jinja2 variable called og_image_url. I imagine this file is served from a bucket or the like in production, so upon deployment that variable should be replaced with the original fallback URL.

The base URL of the OG images is configurable, and should work with either a local or public instance of og-loc, or, as suggested in the related discussion, a file in a bucket.

Is this going in the right direction?

To do:

  • cache rendered HTML with moka

@hdoordt hdoordt marked this pull request as draft January 28, 2025 12:57
@hdoordt hdoordt force-pushed the main branch 7 times, most recently from 27b2101 to 1c92824 Compare January 28, 2025 14:55
@hdoordt hdoordt marked this pull request as ready for review January 28, 2025 15:01
src/config/server.rs Outdated Show resolved Hide resolved
src/middleware/ember_html.rs Outdated Show resolved Hide resolved
src/middleware/ember_html.rs Outdated Show resolved Hide resolved
src/middleware/ember_html.rs Outdated Show resolved Hide resolved
src/middleware/ember_html.rs Outdated Show resolved Hide resolved
@hdoordt hdoordt force-pushed the main branch 5 times, most recently from e45d62a to 77fbee2 Compare February 4, 2025 12:49
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

I've rebased and simplified the PR a little bit more. Let's give this a go and see how it behaves on the server.

@Turbo87 Turbo87 merged commit 223ccc1 into rust-lang:main Feb 7, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants