Skip to content

Commit

Permalink
Fix render during interactive readyState (#1151)
Browse files Browse the repository at this point in the history
  • Loading branch information
rakelley authored and justin808 committed Oct 3, 2018
1 parent 56bec6c commit 46350ce
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac
Changes since last non-beta release.

*Please add entries here for your pull requests that are not yet released.*
- Fix client startup invoking render prematurely. Closes [issue #1150](https://github.com/shakacode/react_on_rails/issues/1150).

### [11.1.4] - 2018-09-12

Expand Down
4 changes: 1 addition & 3 deletions node_package/src/clientStartup.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,7 @@ export function clientStartup(context) {

window.setTimeout(() => {
if (!turbolinksInstalled() || !turbolinksSupported()) {
if (document.readyState === 'complete' ||
(document.readyState !== 'loading' && !document.documentElement.doScroll)
) {
if (document.readyState === 'complete') {
debugTurbolinks(
'NOT USING TURBOLINKS: DOM is already loaded, calling reactOnRailsPageLoaded',
);
Expand Down

3 comments on commit 46350ce

@mmaloon
Copy link

@mmaloon mmaloon commented on 46350ce Oct 6, 2018

Choose a reason for hiding this comment

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

With this update my app no longer renders. The document.readyState === 'interactive' but the 'DOMContentLoaded' event has already fired, so reactOnRailsPageLoaded never gets called :( reverting back to 11.1.4. (Testing on Chrome 69.0.3497.100)

@justin808
Copy link
Member

Choose a reason for hiding this comment

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

@mmaloon I just merged #1152

I'm going to push a release now.

@rakelley
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mmaloon @justin808 Apologies, that was completely my fault, if a page had enough un-preloaded images or iframes the window.setTimeout I left unchanged would sometimes drop invocation into the dead zone between DOMContentLoaded and readyState === complete. #1152 underwent better internal testing on a real-world project and should have no issues because state detection isn't deferred, only the actual rendering is.

Please sign in to comment.