Skip to content

GetMatchStats should use hydration status#9007

Merged
sophie-macmillan merged 2 commits intomainfrom
sm-fix-match-stats
Oct 5, 2023
Merged

GetMatchStats should use hydration status#9007
sophie-macmillan merged 2 commits intomainfrom
sm-fix-match-stats

Conversation

@sophie-macmillan
Copy link
Copy Markdown
Contributor

@sophie-macmillan sophie-macmillan commented Oct 4, 2023

What does this change?

This updates the Placeholder component to check the hydrationStatus and return the Loading placeholder if not hydrated

Why?

The MatchStats component was breaking when there were a lot of names, this is because the serverside rendered comonent wasn't tall enough. Quick fix is to render the placeholder if not hydrated.

Fixes #9006

Screenshots

Before After
image image

@sophie-macmillan sophie-macmillan added the run_chromatic Runs chromatic when label is applied label Oct 4, 2023
@sophie-macmillan sophie-macmillan requested a review from a team as a code owner October 4, 2023 08:03
@sophie-macmillan sophie-macmillan changed the title Placeholder should use min-height rather than height GetMatchStats should use hydration status Oct 4, 2023
}>(matchUrl, options);

if (loading) return <Loading />;
if (loading || !hydrated) return <Loading />;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The other option for a fix here is to update the Placeholder component to use a min-height rather than a height.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I actually think approach is more robust as it addressed the underlying problem of rehydration mismatches.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 4, 2023

Size Change: 0 B 🆕

Total Size: 0 B

compressed-size-action

@sophie-macmillan
Copy link
Copy Markdown
Contributor Author

Closing as superseded by #9008, which allows us to still serve the component from the server

@sophie-macmillan sophie-macmillan deleted the sm-fix-match-stats branch October 4, 2023 12:42
@sophie-macmillan sophie-macmillan restored the sm-fix-match-stats branch October 5, 2023 07:32
@sophie-macmillan
Copy link
Copy Markdown
Contributor Author

Reopening, as we have had other reports of issues with the GetMatchStat component, which will be fixed by this PR

@sophie-macmillan sophie-macmillan added run_chromatic Runs chromatic when label is applied and removed run_chromatic Runs chromatic when label is applied labels Oct 5, 2023
@sophie-macmillan sophie-macmillan merged commit f0f4d2f into main Oct 5, 2023
@sophie-macmillan sophie-macmillan deleted the sm-fix-match-stats branch October 5, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dotcom-rendering run_chromatic Runs chromatic when label is applied

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Match Stats breaking

4 participants