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

First pass of prerender-default hybrid rendering #3259

Merged
merged 6 commits into from
May 18, 2023
Merged

Conversation

matthewp
Copy link
Contributor

What kind of changes does this PR include?

  • New or updated content

Description

@netlify
Copy link

netlify bot commented May 15, 2023

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 01b16c9
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/64665655f17eef0008ffdc6e
😎 Deploy Preview https://deploy-preview-3259--astro-docs-2.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 settings.

@matthewp
Copy link
Contributor Author

Is this a good place? I thought about merging it with the other prerender docs and putting them all together, but given that this is currently experimental it felt weird to do that.

@sarah11918
Copy link
Member

sarah11918 commented May 15, 2023

Docs look pretty good Matthew, I'll just need a quick edit, I'm sure. Placement is perfect!

In light of these new docs, though, I might actually want to change some of the hybrid rendering section itself. Right now the page tells a story like:

We have Hybrid Rendering! Enable it on an individual page that needs pre-rendering.

We also have a setting output: hybrid that does the opposite of what we just said hybrid rendering is!

But you're right, since the new stuff is still experimental, we would want to keep them separate. Let me take a pass at this section as a whole!

@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) labels May 15, 2023
Just an idea of how the two parts could maybe flow better together?
@sarah11918
Copy link
Member

@Yan-Thomas Are you available to go over this closely? 😄

Copy link
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

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

Nice work @matthewp, there's a couple suggestions but after tackling these, LGTM!

src/content/docs/en/guides/server-side-rendering.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/server-side-rendering.mdx Outdated Show resolved Hide resolved
matthewp and others added 2 commits May 15, 2023 18:34
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
@sarah11918
Copy link
Member

Alright, now that Yan has looked at this, I declare it ready to merge on release! 🥳

@sarah11918 sarah11918 merged commit a9d9026 into main May 18, 2023
@sarah11918 sarah11918 deleted the hybrid-rendering branch May 18, 2023 16:56
@sarah11918 sarah11918 added the minor-release For the next minor release; in the milestone, "merge queue" when approved by Sarah! label Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" when approved by Sarah!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hybrid rendering
3 participants