-
Notifications
You must be signed in to change notification settings - Fork 1.5k
use async helmet #5785
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
base: main
Are you sure you want to change the base?
use async helmet #5785
Conversation
CodSpeed Performance ReportMerging #5785 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR migrates Reflex from the legacy react-helmet
library to [email protected]
to ensure compatibility with modern React versions (18+) and concurrent rendering features. The migration introduces several key changes:
Core Library Migration: The helmet component now uses [email protected]
instead of [email protected]
, updating both the Python component definition and the frontend dependency in the installer constants.
New HelmetProvider Component: A new HelmetProvider
class has been added to the helmet module, which is required by the async library to provide context for all helmet operations. This provider must wrap the entire application tree.
Automatic App Wrapping: The most significant architectural change is in app.py
, where HelmetProvider
is now automatically added to the app wrappers at priority 199. This ensures every Reflex application has the necessary helmet context without requiring manual setup from developers.
Updated Public API: The core module exports have been expanded to include both HelmetProvider
class and helmet_provider
factory function, making them available through Reflex's standard component import system.
Test Updates: Unit tests have been updated to expect the new jsx(HelmetProvider,{},...)
wrapper in compiled output, reflecting the new component hierarchy.
This migration fits into Reflex's architecture by leveraging the existing app wrapper system to transparently handle the provider requirement, maintaining backward compatibility while enabling modern React features. The change ensures that existing helmet usage continues to work while providing the foundation for async document head management.
Confidence score: 4/5
- This PR is generally safe to merge with some considerations for production deployment
- Score reflects a well-executed library migration with proper architectural integration, but the breaking nature of the underlying library change requires careful validation
- Pay close attention to the app.py changes and ensure comprehensive testing of helmet functionality across different scenarios
6 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, i thought the tests were passing when i clicked approve 🤔
No description provided.