-
-
Notifications
You must be signed in to change notification settings - Fork 638
Postpone hydration to the next JS task to fix a bug in safari #1729
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
Postpone hydration to the next JS task to fix a bug in safari #1729
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant Doc as Document
participant Startup as clientStartup
participant Callback as Callback
Note over Startup: Document readyState is "complete"
Doc->>Startup: onPageReady(callback)
Startup->>Callback: setTimeout(callback, 0)
Note right of Callback: Execution deferred until after the load event
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
21-24: Suggestion: Enhance Changelog Entry for Clarity
The entry currently states that the bug where theloadevent was not firing in Safari has been fixed. Since the underlying fix involves postponing hydration to the next JavaScript task (usingsetTimeout(callback, 0)), it might be helpful to mention that detail for added clarity. This additional context can provide valuable insight for users and maintainers reviewing the changelog entry.Optional diff suggestion:
- - Fixed a bug where the `load` event was not firing in Safari. [PR 1729](https://github.com/shakacode/react_on_rails/pull/1729) by [Romex91](https://github.com/Romex91). + - Fixed a bug where the `load` event was not firing in Safari by postponing hydration to the next JavaScript task using `setTimeout(callback, 0)`. [PR 1729](https://github.com/shakacode/react_on_rails/pull/1729) by [Romex91](https://github.com/Romex91).
Summary
When hydrating React from the
readystatechangelistener, Safari skips theloadevent.Not sure why, but looks like a bug in the browser.
This issue caused the Web Builder to load infinitely in Popmenu
Pull Request checklist
Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by
~.Add the CHANGELOG entry at the top of the file.
Other Information
Remove this paragraph and mention any other important and relevant information such as benchmarks.
This change is
Summary by CodeRabbit
loadevent not firing in Safari in the CHANGELOG.ReactOnRailsmodule and thereact-on-railspackage from "14.2.0" to "14.2.1".