From 7588fc3c242e2eb9da545455f13eccced159cc96 Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Sat, 27 May 2023 13:22:48 -0500 Subject: [PATCH 1/6] only use setTimeout on readyState=complete --- node_package/src/clientStartup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_package/src/clientStartup.ts b/node_package/src/clientStartup.ts index f4aae5517f..2c59d998bb 100644 --- a/node_package/src/clientStartup.ts +++ b/node_package/src/clientStartup.ts @@ -309,7 +309,7 @@ export function clientStartup(context: Context): void { // but sub-resources such as images, stylesheets and frames are still loading. // If lazy asynch loading is used, such as with loadable-components, then the init // function will install some handler that will properly know when to do hyrdation. - if (document.readyState !== 'loading') { + if (document.readyState == 'complete') { window.setTimeout(renderInit); } else { document.addEventListener('DOMContentLoaded', renderInit); From 63dc80d7364ea6f77c6474922241c0984bd1f0d1 Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Sat, 27 May 2023 18:39:46 -0500 Subject: [PATCH 2/6] linting --- node_package/src/clientStartup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_package/src/clientStartup.ts b/node_package/src/clientStartup.ts index 2c59d998bb..d06ffaa10a 100644 --- a/node_package/src/clientStartup.ts +++ b/node_package/src/clientStartup.ts @@ -309,7 +309,7 @@ export function clientStartup(context: Context): void { // but sub-resources such as images, stylesheets and frames are still loading. // If lazy asynch loading is used, such as with loadable-components, then the init // function will install some handler that will properly know when to do hyrdation. - if (document.readyState == 'complete') { + if (document.readyState === 'complete') { window.setTimeout(renderInit); } else { document.addEventListener('DOMContentLoaded', renderInit); From eab6d7fb7943a9788938312790ddc8f4badd872a Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Sat, 27 May 2023 18:45:39 -0500 Subject: [PATCH 3/6] reenable defer for spec/dummy --- spec/dummy/app/views/layouts/application.html.erb | 2 +- spec/dummy/config/initializers/react_on_rails.rb | 1 - spec/dummy/spec/helpers/react_on_rails_helper_spec.rb | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/dummy/app/views/layouts/application.html.erb b/spec/dummy/app/views/layouts/application.html.erb index 587795ef31..ca89d84d47 100644 --- a/spec/dummy/app/views/layouts/application.html.erb +++ b/spec/dummy/app/views/layouts/application.html.erb @@ -10,7 +10,7 @@ <%= yield :head %> - <%= javascript_pack_tag('client-bundle', 'data-turbolinks-track': true, defer: false) %> + <%= javascript_pack_tag('client-bundle', 'data-turbolinks-track': true) %> <%= csrf_meta_tags %> diff --git a/spec/dummy/config/initializers/react_on_rails.rb b/spec/dummy/config/initializers/react_on_rails.rb index 234a4ea999..8f3f247cc5 100644 --- a/spec/dummy/config/initializers/react_on_rails.rb +++ b/spec/dummy/config/initializers/react_on_rails.rb @@ -36,5 +36,4 @@ def self.adjust_props_for_client_side_hydration(component_name, props) config.rendering_props_extension = RenderingPropsExtension config.components_subdirectory = "startup" config.auto_load_bundle = true - config.defer_generated_component_packs = false end diff --git a/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb b/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb index 3ff4a9414f..dbe901ea74 100644 --- a/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb +++ b/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb @@ -42,7 +42,7 @@ class PlainReactOnRailsHelper allow(helper).to receive(:append_javascript_pack_tag) allow(helper).to receive(:append_stylesheet_pack_tag) expect { helper.load_pack_for_generated_component("component_name") }.not_to raise_error - expect(helper).to have_received(:append_javascript_pack_tag).with("generated/component_name", { defer: false }) + expect(helper).to have_received(:append_javascript_pack_tag).with("generated/component_name", { defer: true }) expect(helper).to have_received(:append_stylesheet_pack_tag).with("generated/component_name") end end From 6f427bc15097ff6a1d36ab5dd877a695f543def1 Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Sat, 27 May 2023 18:50:45 -0500 Subject: [PATCH 4/6] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0022c981a9..0b1ce8cdab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ Changes since last non-beta release. *Please add entries here for your pull requests that are not yet released.* +- Fixed race condition where a react component could attempt to initialize before it had been registered. [PR 1540](https://github.com/shakacode/react_on_rails/pull/1540) by [judahmeek](https://github.com/judahmeek) + ### [13.3.4] - 2022-05-23 ### Added From 987de823d271236c74382d11315c0535a8216b9d Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Sat, 27 May 2023 21:18:25 -0500 Subject: [PATCH 5/6] remove comments --- node_package/src/clientStartup.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/node_package/src/clientStartup.ts b/node_package/src/clientStartup.ts index d06ffaa10a..4d784e88be 100644 --- a/node_package/src/clientStartup.ts +++ b/node_package/src/clientStartup.ts @@ -304,11 +304,6 @@ export function clientStartup(context: Context): void { debugTurbolinks('Adding DOMContentLoaded event to install event listeners.'); - // So long as the document is not loading, we can assume: - // The document has finished loading and the document has been parsed - // but sub-resources such as images, stylesheets and frames are still loading. - // If lazy asynch loading is used, such as with loadable-components, then the init - // function will install some handler that will properly know when to do hyrdation. if (document.readyState === 'complete') { window.setTimeout(renderInit); } else { From a46fc0f685bed1b22120d3e733a32ec398493627 Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Tue, 30 May 2023 17:29:12 -0500 Subject: [PATCH 6/6] changes per review --- node_package/src/clientStartup.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/node_package/src/clientStartup.ts b/node_package/src/clientStartup.ts index 4d784e88be..375e610a74 100644 --- a/node_package/src/clientStartup.ts +++ b/node_package/src/clientStartup.ts @@ -286,6 +286,17 @@ function isWindow(context: Context): context is Window { return (context as Window).document !== undefined; } +function onPageReady(callback: () => void) { + if (document.readyState === "complete") { + callback(); + } else { + document.addEventListener("readystatechange", function onReadyStateChange() { + onPageReady(callback); + document.removeEventListener("readystatechange", onReadyStateChange); + }); + } +} + export function clientStartup(context: Context): void { // Check if server rendering if (!isWindow(context)) { @@ -302,11 +313,5 @@ export function clientStartup(context: Context): void { // eslint-disable-next-line no-underscore-dangle, no-param-reassign context.__REACT_ON_RAILS_EVENT_HANDLERS_RAN_ONCE__ = true; - debugTurbolinks('Adding DOMContentLoaded event to install event listeners.'); - - if (document.readyState === 'complete') { - window.setTimeout(renderInit); - } else { - document.addEventListener('DOMContentLoaded', renderInit); - } + onPageReady(renderInit); }