-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
feat(frontend/hooks): replace 3rd-party BroadcastChannel with native Web API equivalence #29584
feat(frontend/hooks): replace 3rd-party BroadcastChannel with native Web API equivalence #29584
Conversation
…Web API equivalent Signed-off-by: hainenber <[email protected]>
// After that, we should remove this dependency and use the native API. | ||
const channel = new BroadcastChannel<TabIdChannelMessage>('tab_id_channel'); | ||
const channel: StrictBroadcastChannel<TabIdChannelMessage> = | ||
new BroadcastChannel('tab_id_channel'); |
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.
nit: How about making the tab_id_channel
value constant?
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.
done in HEAD :D
Thanks for tackling this! This looks like a great improvement, and long overdue! Is there already (or should there be) a test making sure that messages are sent/received correctly? |
Signed-off-by: hainenber <[email protected]>
Unfortunately there's none but I can produce the expected result similar to what described in the original PR :D State key is kept between reloads for a dashboard/chart opened in Explore tab. Would be sufficient? Screen.Recording.2024-07-17.at.23.13.23.mov |
Signed-off-by: hainenber <[email protected]>
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.
It might be nice to have added test coverage, but this is an improvement over the current state of affairs for the product, and moves away from package complexities toward native browser, support, so I call it a win :)
…Web API equivalence (#29584) Signed-off-by: hainenber <[email protected]>
feat(frontend/hooks): replace 3rd-party BroadcastChannel usage with native Web API equivalence
SUMMARY
Implementation of 2 years old TODO :D
Safari 15.4 was released 2 years ago (source) and so we can utilize its native Web API to replace a 3rd-party implementation.
Credit to Eric Wong's SO post for the implementation. I'm just a copy-paster after all :D
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
All CI checks should pass
ADDITIONAL INFORMATION