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

Edit: make RSS canonicalUrl required #453

Merged
merged 5 commits into from
May 5, 2022

Conversation

bholmesdev
Copy link
Contributor

@bholmesdev bholmesdev commented May 5, 2022

We discovered that canonicalUrl did not work as expected in the @astrojs/rss package. So, we're asking users to apply this value manually as a required prop.

Changes made

  • rename canonicalUrl option to site
  • add note that import.meta.env.SITE only works in the latest 1.0 beta
  • add site to all code examples, with explainer comments where applicable

@netlify
Copy link

netlify bot commented May 5, 2022

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 29222bc
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/6274495aa63eb000082e98a5
😎 Deploy Preview https://deploy-preview-453--astro-docs-2.netlify.app/kr/core-concepts/partial-hydration
📱 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 settings.

@sarah11918
Copy link
Member

import.meta.env.HELL -- I will check the deploy preview when it's ready, fix this if it needs, in one go, and with ALL the proper surrounding HTML required to make the component work. :/

@bholmesdev
Copy link
Contributor Author

bholmesdev commented May 5, 2022

@sarah11918 Deploy preview looks fine to me! Shouldn't be a problem in a code fence from my understanding, since Shiki chops up the import into spans for syntax highlighting. Lmk if there's something crazy I don't know about for production though

@sarah11918
Copy link
Member

@bholmesdev Gah! I replied in the other PR by mistake... short version, this is what this current Netlify deploy preview looks like:

Can you look carefully at this? Because this is what I see:

When it's in commented out code in the code fence, you're gonna need that component (as I gave instructions for dealing with, in the other PR):
Screenshot 2022-05-05 18 35 05

When it's just a string in the code fence, it seems to be fine:
Screenshot 2022-05-05 18 39 03

sarah11918 and others added 3 commits May 5, 2022 17:55
@bholmesdev - please check this, and adjust as necessary to get it to render properly.  This might require fiddling, but the ideas are:

1. Where ever you're seeing {{}} in the preview render instead of import.meta.env, then you need to import (I've done) and use (I've done) this component instead of typing the phrase out directly. The rest of the expression can be passed as the`path=" "` attribute, and it will be formatted as code.

2. This component must be inside an HTML element, and can't be written inside regular Markdown. I've tried the simplest version first (just <p> tags around the line it's in). If this isn't sufficient, or screws up the rest of the rendering, then you'll have to "work your way out" converting larger and larger chunks to HTML until you have the rendering you need.

(Or, you figure out whether it's ABSOLUTELY NECESSARY to write the phrase import.meta.env. Dealer's choice.) Hope this helps.

Co-authored-by: Ben Holmes <[email protected]>
@bholmesdev
Copy link
Contributor Author

Totally understood! Thanks for walking through this nuanced / wacky issue @sarah11918 🙏

@bholmesdev bholmesdev merged commit cb3684b into main May 5, 2022
@bholmesdev bholmesdev deleted the fix/rss-canonical-url-required branch May 5, 2022 22:21
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.

2 participants