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

Use GitHub Pages site origin for setting up SSG configs #21

Merged
merged 2 commits into from Sep 11, 2022

Conversation

AndrewLester
Copy link
Contributor

From what I could tell, two (SvelteKit and Gatsby) of the currently supported static site generators have a configuration option for the URL of the deployed site. This allows for code to access the site URL during prerendering, which is useful for social sharing, meta tags, JSON-LD, and more. Since configure-pages does have knowledge of this URL, I think it makes sense to pass it to the SSG configurations.

Let me know if this was already discussed and decided against. Also, the other generators might support a similar config option, and I would be happy to add them to this PR.

I got the name for Gatsby's config option from the docs. It's possible that users would want the pathPrefix also attached to the siteUrl, which is certainly possible. I didn't include it because it is accessible with library functions and in the previous tests with a siteURL (this was the capitalization before), it didn't include the pathPrefix.

@AndrewLester AndrewLester requested a review from a team as a code owner August 19, 2022 20:10
Copy link
Member

@JamesMGreene JamesMGreene left a comment

Choose a reason for hiding this comment

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

Seems reasonable and makes sense.

In Gatsby, the siteMetadata.siteUrl doesn't appear to be a standard configuration, but it does appear to be a pretty common convention in plugins and such, so I'm OK with merging that bit as well. 👍🏻

Thank you! 💝

@JamesMGreene JamesMGreene merged commit 2745795 into actions:main Sep 11, 2022
@JamesMGreene
Copy link
Member

Apologies that this one took a little while to get merged, @AndrewLester. 😅 Thanks again for the contribution.

This is now available in the latest release (v2.1.0). 🚀

@AndrewLester
Copy link
Contributor Author

No worries, thanks for reviewing and merging this!

JamesMGreene added a commit that referenced this pull request Sep 17, 2022
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.

None yet

2 participants