Uh oh!
There was an error while loading. Please reload this page.
Use SSR-compatible slot implementation in PageLayout - #3036
Conversation
🦋 Changeset detectedLatest commit: 5dcc159 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
joshblack
left a comment
There was a problem hiding this comment.
Looks great 🔥 Just left some comments for some ideas while going through the code, feel free to ignore if they're not helpful 👍
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
siddharthkp
commented
Mar 16, 2023
Hi! Just leaving it here in case we need it: In case this change isn't backward compatible with instances of PageLayout in gh/gh, we can unblock this team by putting this in drafts/experimental first. |
colebemis
commented
Mar 16, 2023
Uh oh!
There was an error while loading. Please reload this page.
Problem
PageLayout.HeaderandPageLayout.Footerdon't render server-side because PageLayout uses an SSR-incompatible approach to implement its slots API (issue).Screen.Recording.2023-03-14.at.2.58.43.PM.mov
Notice that the layout shifts because the header appears after the page loads.
Slack discussion for context
Solution
This PR updates PageLayout to use an SSR-compatible slots implementation.
Tradeoff
In this new implementation,
PageLayout.HeaderandPageLayout.Footermust be direct children ofPageLayout. This feels like a reasonable tradeoff becausePageLayout.HeaderandPageLayout.Footeraren't used often, and when they are used, they're almost always a direct child ofPageLayout.PageLayout.Headerin 3 places. In 2 of 3 usages,PageLayout.Headeris a direct child ofPageLayout. In the 3rd case,PageLayout.Headercan easily be refactored to be direct child ofPageLayout.PageLayout.Footer.Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.