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

Dedupe legacy context warnings #30299

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Dedupe legacy context warnings #30299

merged 1 commit into from
Jul 9, 2024

Conversation

kassens
Copy link
Member

@kassens kassens commented Jul 9, 2024

Similar to other warnings about legacy APIs, only raise a warning once per component.

.

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link

vercel bot commented Jul 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 9, 2024 7:33pm

Similar to other warnings about legacy APIs, only raise a warning once per component.

.
@@ -105,7 +104,7 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
<RegularFn />
</span>
</LegacyProvider>,
render === clientRenderOnBadMarkup ? 4 : 3,
3,
Copy link
Member Author

Choose a reason for hiding this comment

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

The different number of warnings originates from this PR, but didn't have a reason there. I assume it was just updated to make the test pass?

https://github.com/facebook/react/pull/28448/files#diff-e378a74630afcd49e831e7376016c653c5dcf045308d7a9142a1eb912f603bfbR108

Copy link
Collaborator

Choose a reason for hiding this comment

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

On hydration mismatch we retry with client rendering in the new root so you got the warning again since it wasn't deduped.

That we now have the same count with good and bad markup shows that it's properly deduped 👍🏻

I'll add a comment to the original PR. There were a bunch of PR with the same pattern back then so I just forgot to mention it in each one.

@@ -105,7 +104,7 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => {
<RegularFn />
</span>
</LegacyProvider>,
render === clientRenderOnBadMarkup ? 4 : 3,
3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

On hydration mismatch we retry with client rendering in the new root so you got the warning again since it wasn't deduped.

That we now have the same count with good and bad markup shows that it's properly deduped 👍🏻

I'll add a comment to the original PR. There were a bunch of PR with the same pattern back then so I just forgot to mention it in each one.

@kassens kassens merged commit 39e69dc into facebook:main Jul 9, 2024
256 checks passed
@kassens kassens deleted the pr30299 branch July 9, 2024 23:55
github-actions bot pushed a commit that referenced this pull request Jul 10, 2024
Similar to other warnings about legacy APIs, only raise a warning once per component.

DiffTrain build for [39e69dc](39e69dc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants