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

Do not cache share links #757

Merged
merged 4 commits into from
Oct 24, 2024
Merged

Conversation

melroy89
Copy link
Contributor

You can't just cache the share links, that will result into every blog post having the same URL.

Just use partial on the getShareLink.html. So now each post will show the correct URL to share the post via social media or alike.

Signed-off-by: Melroy van den Berg <[email protected]>
@melroy89 melroy89 mentioned this pull request Oct 22, 2024
Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for ananke-theme ready!

Name Link
🔨 Latest commit 47ee7da
🔍 Latest deploy log https://app.netlify.com/sites/ananke-theme/deploys/671a1a18b390640008b2bff8
😎 Deploy Preview https://deploy-preview-757--ananke-theme.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@davidsneighbour
Copy link
Collaborator

False. There is a bug in the logic of the partial, that is true. But we CAN and will cache those links. A share link must be cacheable for a specific page. Check the $options dict. page here might refer to the outer list page a post is listed on, which is what ruins the cache if you implement share links on list pages. What this needs is $context (defined in the first line) instead of page in the options and then they (should be/are) individual caches per post object (if $context doesn't walk into the same trap). I'll need to check that though, it might be that I will inline the sharelink generation.

@davidsneighbour davidsneighbour added the Waiting Waiting on a response or other action label Oct 23, 2024
@davidsneighbour
Copy link
Collaborator

Something along the lines of this, not tested yet:

#763

@davidsneighbour davidsneighbour self-requested a review October 24, 2024 09:50
Signed-off-by: Patrick Kollitsch <[email protected]>
@davidsneighbour
Copy link
Collaborator

I am going to merge this now. I opened an issue for going to look at caching after the release.

#764

Signed-off-by: Patrick Kollitsch <[email protected]>
@davidsneighbour davidsneighbour merged commit 473c7c1 into theNewDynamic:main Oct 24, 2024
4 checks passed
@melroy89 melroy89 deleted the patch-6 branch October 24, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting Waiting on a response or other action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants