From 50944d3884254cb6e7e31b4b4d282b1351773ff6 Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Tue, 17 Aug 2021 22:37:53 -0400 Subject: [PATCH 01/10] Add for upload complete to trigger page reload --- lib/shopify-cli/theme/dev_server.rb | 2 +- .../theme/dev_server/hot-reload.js | 24 ++++++++++++------- .../theme/dev_server/hot_reload.rb | 12 +++++++++- lib/shopify-cli/theme/syncer.rb | 8 +++++++ 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/shopify-cli/theme/dev_server.rb b/lib/shopify-cli/theme/dev_server.rb index 14d7eb82fc..5b731b38c6 100644 --- a/lib/shopify-cli/theme/dev_server.rb +++ b/lib/shopify-cli/theme/dev_server.rb @@ -30,7 +30,7 @@ def start(ctx, root, port: 9292) # Setup the middleware stack. Mimics Rack::Builder / config.ru, but in reverse order @app = Proxy.new(ctx, theme: theme, syncer: @syncer) @app = LocalAssets.new(ctx, @app, theme: theme) - @app = HotReload.new(ctx, @app, theme: theme, watcher: watcher, ignore_filter: ignore_filter) + @app = HotReload.new(ctx, @app, theme: theme, watcher: watcher, syncer: @syncer, ignore_filter: ignore_filter) stopped = false theme.ensure_exists! diff --git a/lib/shopify-cli/theme/dev_server/hot-reload.js b/lib/shopify-cli/theme/dev_server/hot-reload.js index bd90edafbe..f41e7c0c95 100644 --- a/lib/shopify-cli/theme/dev_server/hot-reload.js +++ b/lib/shopify-cli/theme/dev_server/hot-reload.js @@ -17,18 +17,24 @@ connect(); + let waitingForFileUpload = false; function handleUpdate(message) { var data = JSON.parse(message.data); - // Assume only one file is modified at a time - var modified = data.modified[0]; - - if (isCssFile(modified)) { - reloadCssFile(modified) - } else if (isSectionFile(modified)) { - reloadSection(modified); - } else { - console.log(`[HotReload] Refreshing entire page`); + if(data.modified) { + data.modified.forEach(file => { + if (isCssFile(file)) { + reloadCssFile(file) + } else if (isSectionFile(file)) { + reloadSection(file); + } else { + console.log(`[HotReload] Refreshing entire page, waiting for file uploads`); + waitingForFileUpload = true; + } + }); + } + + if(waitingForFileUpload && data.uploadComplete) { window.location.reload(); } } diff --git a/lib/shopify-cli/theme/dev_server/hot_reload.rb b/lib/shopify-cli/theme/dev_server/hot_reload.rb index 3a6cad756d..470479d2eb 100644 --- a/lib/shopify-cli/theme/dev_server/hot_reload.rb +++ b/lib/shopify-cli/theme/dev_server/hot_reload.rb @@ -4,13 +4,15 @@ module ShopifyCli module Theme module DevServer class HotReload - def initialize(ctx, app, theme:, watcher:, ignore_filter: nil) + def initialize(ctx, app, theme:, watcher:, syncer:, ignore_filter: nil) @ctx = ctx @app = app @theme = theme @streams = SSE::Streams.new @watcher = watcher @watcher.add_observer(self, :notify_streams_of_file_change) + @syncer = syncer + @syncer.add_observer(self, :notify_streams_of_upload_complete) @ignore_filter = ignore_filter end @@ -41,6 +43,14 @@ def notify_streams_of_file_change(modified, added, _removed) @ctx.debug("[HotReload] Modified #{files.join(", ")}") end + def notify_streams_of_upload_complete(complete) + @streams.broadcast(JSON.generate( + uploadComplete: complete + )) + + @ctx.debug("[HotReload] upload complete") + end + private def request_is_html?(headers) diff --git a/lib/shopify-cli/theme/syncer.rb b/lib/shopify-cli/theme/syncer.rb index 08e92fefe7..535650fee3 100644 --- a/lib/shopify-cli/theme/syncer.rb +++ b/lib/shopify-cli/theme/syncer.rb @@ -2,10 +2,13 @@ require "thread" require "json" require "base64" +require "observer" module ShopifyCli module Theme class Syncer + include Observable + class Operation < Struct.new(:method, :file) def to_s "#{method} #{file&.relative_path}" @@ -218,6 +221,11 @@ def perform(operation) ) ensure @pending.delete(operation) + + if @pending.size == 0 + changed + notify_observers(true); + end end def update(file) From 5ccc3c29b230f1ccda1e158440f6135ffdad27de Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Tue, 17 Aug 2021 22:54:28 -0400 Subject: [PATCH 02/10] Add variable to make notify_observers clearer --- lib/shopify-cli/theme/syncer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/shopify-cli/theme/syncer.rb b/lib/shopify-cli/theme/syncer.rb index 535650fee3..1c2b699269 100644 --- a/lib/shopify-cli/theme/syncer.rb +++ b/lib/shopify-cli/theme/syncer.rb @@ -224,7 +224,8 @@ def perform(operation) if @pending.size == 0 changed - notify_observers(true); + uploadComplete = true + notify_observers(uploadComplete); end end From 4d31fa2e1a5bbaf948f5b6601ad56d66ced1188b Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Tue, 17 Aug 2021 23:04:40 -0400 Subject: [PATCH 03/10] Move hotreload log to not be logged multiple times --- lib/shopify-cli/theme/dev_server/hot-reload.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/shopify-cli/theme/dev_server/hot-reload.js b/lib/shopify-cli/theme/dev_server/hot-reload.js index f41e7c0c95..d64cb04277 100644 --- a/lib/shopify-cli/theme/dev_server/hot-reload.js +++ b/lib/shopify-cli/theme/dev_server/hot-reload.js @@ -28,11 +28,14 @@ } else if (isSectionFile(file)) { reloadSection(file); } else { - console.log(`[HotReload] Refreshing entire page, waiting for file uploads`); waitingForFileUpload = true; } }); } + + if(waitingForFileUpload) { + console.log(`[HotReload] Refreshing entire page, waiting for file uploads`); + } if(waitingForFileUpload && data.uploadComplete) { window.location.reload(); From 77d1d8c89395dd0e9d1d777103f11dfb333cfbea Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Tue, 17 Aug 2021 23:30:00 -0400 Subject: [PATCH 04/10] Adds the wait for unsupported section reload --- lib/shopify-cli/theme/dev_server/hot-reload.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/shopify-cli/theme/dev_server/hot-reload.js b/lib/shopify-cli/theme/dev_server/hot-reload.js index d64cb04277..0363921e13 100644 --- a/lib/shopify-cli/theme/dev_server/hot-reload.js +++ b/lib/shopify-cli/theme/dev_server/hot-reload.js @@ -89,7 +89,7 @@ console.log(`[HotReload] Reloaded ${this.name} section`); } else { - window.location.reload() + waitingForFileUpload = true; console.log(`[HotReload] Hot-reloading not supported, fully reloading ${this.name} section`); } From 637350cc14c70e8393b76776f5f188c1c18792bc Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Wed, 6 Oct 2021 13:35:27 -0400 Subject: [PATCH 05/10] Add a test for the upload complete broadcast event --- .../theme/dev_server/hot_reload_test.rb | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/test/shopify-cli/theme/dev_server/hot_reload_test.rb b/test/shopify-cli/theme/dev_server/hot_reload_test.rb index 804bccfa81..ce612c0a61 100644 --- a/test/shopify-cli/theme/dev_server/hot_reload_test.rb +++ b/test/shopify-cli/theme/dev_server/hot_reload_test.rb @@ -12,7 +12,8 @@ def setup root = ShopifyCLI::ROOT + "/test/fixtures/theme" @ctx = TestHelpers::FakeContext.new(root: root) @theme = Theme.new(@ctx, root: root) - @syncer = stub("Syncer", enqueue_uploads: true) + @syncer = Syncer.new(@ctx, theme: @theme) + @syncer.start_threads @watcher = Watcher.new(@ctx, theme: @theme, syncer: @syncer) end @@ -67,7 +68,7 @@ def test_broadcasts_watcher_events .with(JSON.generate(modified: modified)) app = -> { [200, {}, []] } - HotReload.new(@ctx, app, theme: @theme, watcher: @watcher) + HotReload.new(@ctx, app, theme: @theme, watcher: @watcher, syncer: @syncer) @watcher.changed @watcher.notify_observers(modified, [], []) @@ -87,6 +88,7 @@ def test_doesnt_broadcast_watcher_events_when_the_list_is_empty @ctx, app, theme: @theme, watcher: @watcher, + syncer: @syncer, ignore_filter: ignore_filter ) @@ -94,13 +96,25 @@ def test_doesnt_broadcast_watcher_events_when_the_list_is_empty @watcher.notify_observers(modified, [], []) end + def test_broadcast_upload_complete_event + SSE::Streams.any_instance + .expects(:broadcast) + .with(JSON.generate(uploadComplete: true)) + + app = -> { [200, {}, []] } + HotReload.new(@ctx, app, theme: @theme, watcher: @watcher, syncer: @syncer) + + @syncer.enqueue_updates([@theme["snippets/snippet.liquid"]]) + @syncer.wait! + end + private def serve(response_body = "", path: "/", headers: {}) app = lambda do |_env| [200, headers, [response_body]] end - stack = HotReload.new(@ctx, app, theme: @theme, watcher: @watcher) + stack = HotReload.new(@ctx, app, theme: @theme, watcher: @watcher, syncer: @syncer) request = Rack::MockRequest.new(stack) request.get(path).body end From 581113dafef42fe99014cc114b936c04afc978a6 Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Fri, 8 Oct 2021 09:58:00 -0400 Subject: [PATCH 06/10] Change variable name to make it clearer --- lib/shopify_cli/theme/dev_server/hot-reload.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/shopify_cli/theme/dev_server/hot-reload.js b/lib/shopify_cli/theme/dev_server/hot-reload.js index 0363921e13..c5db738f50 100644 --- a/lib/shopify_cli/theme/dev_server/hot-reload.js +++ b/lib/shopify_cli/theme/dev_server/hot-reload.js @@ -17,7 +17,7 @@ connect(); - let waitingForFileUpload = false; + let nonDynamicFileChanged = false; function handleUpdate(message) { var data = JSON.parse(message.data); @@ -28,16 +28,16 @@ } else if (isSectionFile(file)) { reloadSection(file); } else { - waitingForFileUpload = true; + nonDynamicFileChanged = true; } }); } - if(waitingForFileUpload) { + if(nonDynamicFileChanged) { console.log(`[HotReload] Refreshing entire page, waiting for file uploads`); } - if(waitingForFileUpload && data.uploadComplete) { + if(nonDynamicFileChanged && data.uploadComplete) { window.location.reload(); } } @@ -89,7 +89,7 @@ console.log(`[HotReload] Reloaded ${this.name} section`); } else { - waitingForFileUpload = true; + nonDynamicFileChanged = true; console.log(`[HotReload] Hot-reloading not supported, fully reloading ${this.name} section`); } From a4ff5c3031fb38a3a1dde17c6f63e3d295b5589d Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Mon, 25 Oct 2021 19:25:38 -0400 Subject: [PATCH 07/10] Remove unnecessary variable assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marc-André Cournoyer --- lib/shopify_cli/theme/syncer.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/shopify_cli/theme/syncer.rb b/lib/shopify_cli/theme/syncer.rb index 8286d55aaf..f6eb3e1773 100644 --- a/lib/shopify_cli/theme/syncer.rb +++ b/lib/shopify_cli/theme/syncer.rb @@ -224,8 +224,7 @@ def perform(operation) if @pending.size == 0 changed - uploadComplete = true - notify_observers(uploadComplete); + notify_observers(true) end end From 8af899caa331cc15c734a3ad037412d7a2534640 Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Mon, 25 Oct 2021 22:43:43 -0400 Subject: [PATCH 08/10] Add reload for js files --- lib/shopify_cli/theme/dev_server/hot-reload.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/shopify_cli/theme/dev_server/hot-reload.js b/lib/shopify_cli/theme/dev_server/hot-reload.js index c5db738f50..ac42b30cfb 100644 --- a/lib/shopify_cli/theme/dev_server/hot-reload.js +++ b/lib/shopify_cli/theme/dev_server/hot-reload.js @@ -22,19 +22,28 @@ var data = JSON.parse(message.data); if(data.modified) { + let containsJSFiles = false; + data.modified.forEach(file => { if (isCssFile(file)) { reloadCssFile(file) } else if (isSectionFile(file)) { reloadSection(file); + } else if (isJSFile(file)) { + containsJSFiles = true; } else { nonDynamicFileChanged = true; } }); + + if(containsJSFiles && !nonDynamicFileChanged) { + console.log(`[HotReload] Refreshing entire page`); + return window.location.reload(); + } } if(nonDynamicFileChanged) { - console.log(`[HotReload] Refreshing entire page, waiting for file uploads`); + console.log(`[HotReload] Refreshing entire page, waiting for files to upload`); } if(nonDynamicFileChanged && data.uploadComplete) { @@ -42,6 +51,10 @@ } } + function isJSFile(filename) { + return filename.endsWith('.js'); + } + function isCssFile(filename) { return filename.endsWith('.css'); } From d5a621721d95a8c9e74f7da4892f350f772fb3db Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Tue, 26 Oct 2021 16:52:01 -0400 Subject: [PATCH 09/10] Change test to assert includes now that "uploadComplete" can be present --- test/shopify-cli/theme/dev_server/integration_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/shopify-cli/theme/dev_server/integration_test.rb b/test/shopify-cli/theme/dev_server/integration_test.rb index dbca39dd29..5f248a6fbe 100644 --- a/test/shopify-cli/theme/dev_server/integration_test.rb +++ b/test/shopify-cli/theme/dev_server/integration_test.rb @@ -164,7 +164,7 @@ def test_streams_hot_reload_events file = Pathname.new("#{ShopifyCLI::ROOT}/test/fixtures/theme/assets/theme.css") file.write("modified") begin - assert_equal("2a\r\ndata: {\"modified\":[\"assets/theme.css\"]}\n\n\n\r\n", socket.readpartial(1024)) + assert_includes(socket.readpartial(1024), "2a\r\ndata: {\"modified\":[\"assets/theme.css\"]}\n\n\n\r\n") ensure file.write("") end From f2d34e1acd06d8478519253833b4efcfdadfc11a Mon Sep 17 00:00:00 2001 From: Tommy Gaudreau Date: Tue, 26 Oct 2021 16:53:10 -0400 Subject: [PATCH 10/10] remove rubocop trailing space --- test/shopify-cli/theme/dev_server/hot_reload_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/shopify-cli/theme/dev_server/hot_reload_test.rb b/test/shopify-cli/theme/dev_server/hot_reload_test.rb index ce612c0a61..dab64f3166 100644 --- a/test/shopify-cli/theme/dev_server/hot_reload_test.rb +++ b/test/shopify-cli/theme/dev_server/hot_reload_test.rb @@ -103,7 +103,7 @@ def test_broadcast_upload_complete_event app = -> { [200, {}, []] } HotReload.new(@ctx, app, theme: @theme, watcher: @watcher, syncer: @syncer) - + @syncer.enqueue_updates([@theme["snippets/snippet.liquid"]]) @syncer.wait! end