Skip to content
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.

Sign up for GitHub

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

[3.46] Fix an issue in shopify theme dev and shopify app dev that was affecting assets/fonts loading on local servers #2107

Merged
merged 2 commits into from
Jun 7, 2023
Merged

[3.46] Fix an issue in shopify theme dev and shopify app dev that was affecting assets/fonts loading on local servers #2107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd7c3f0
Fix an issue in `shopify theme dev` and `shopify app dev` that was af…
karreiro Jun 6, 2023
6d709d7
Fix an issue in `shopify theme dev` that was affecting asset loading …
karreiro Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/blue-carpets-sin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': patch
'@shopify/theme': patch
---

Fix an issue in `shopify theme dev` and `shopify app dev` that was affecting image loading on local servers
6 changes: 6 additions & 0 deletions .changeset/pink-wasps-marry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': patch
'@shopify/theme': patch
---

Fix an issue in `shopify theme dev` that was affecting asset loading on local servers, in some shops
2 changes: 1 addition & 1 deletion packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def start_server

def middleware_stack
@app = Proxy.new(ctx, theme, param_builder)
@app = CdnFonts.new(@app, theme: theme)
@app = CdnFonts.new(ctx, @app, theme: theme)
@app = LocalAssets.new(ctx, @app, theme)
@app = HotReload.new(ctx, @app, broadcast_hooks: broadcast_hooks, watcher: watcher, mode: mode,
script_injector: script_injector)
Expand Down
11 changes: 8 additions & 3 deletions packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ class DevServer
class CdnFonts
FONTS_PATH = "/fonts"
FONTS_CDN = "https://fonts.shopifycdn.com"
FONTS_REGEX = %r{#{FONTS_CDN}}

def initialize(app, theme:)
def initialize(ctx, app, theme:)
@ctx = ctx
@app = app
@theme = theme
end
Expand Down Expand Up @@ -69,7 +69,12 @@ def fonts_cdn_uri(path, query)
end

def replace_font_urls(body)
[body.join.gsub(FONTS_REGEX, FONTS_PATH)]
fonts_regex = %r{#{FONTS_CDN}|((http:|https:)?//#{shop}/cdn/fonts)}
[body.join.gsub(fonts_regex, FONTS_PATH)]
end

def shop
@shop ||= ShopifyCLI::Theme::ThemeAdminAPI.new(@ctx).get_shop_or_abort
end
end
end
Expand Down
17 changes: 13 additions & 4 deletions packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/local_assets.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ module ShopifyCLI
module Theme
class DevServer
class LocalAssets
THEME_REGEX = %r{//cdn\.shopify\.com/s/.+?/(assets/.+?\.(?:css|js))}
VANITY_THEME_REGEX = %r{/cdn/shop/.+?/(assets/.+?\.(?:css|js))}
SUPPORTED_EXTENSIONS = [:jpg, :jpeg, :js, :css, :png, :svg].join("|")
CDN_REGEX = %r{(//cdn)\.shopify\.com/s/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
VANITY_CDN_REGEX = %r{(/cdn)/shop/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}

class FileBody
def initialize(path)
Expand Down Expand Up @@ -42,13 +43,17 @@ def call(env)
end
end

def shop_regex
%r{(http:|https:)?//#{shop}/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
end

private

def replace_asset_urls(body)
replaced_body = body.join
[THEME_REGEX, VANITY_THEME_REGEX].each do |regex|
[CDN_REGEX, VANITY_CDN_REGEX, shop_regex].each do |regex|
replaced_body = replaced_body.gsub(regex) do |match|
path = Regexp.last_match[1]
path = Regexp.last_match[2]
@target.static_asset_paths.include?(path) ? "/#{path}" : match
end
end
Expand Down Expand Up @@ -86,6 +91,10 @@ def serve_file(path_info)
serve_fail(404, "Not found")
end
end

def shop
@shop ||= ShopifyCLI::Theme::ThemeAdminAPI.new(@ctx).get_shop_or_abort
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/extension/dev_server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def start(ctx, root, port: 9292, theme: nil, generate_tmp_theme: false, project:

def middleware_stack
@app = Proxy.new(ctx, theme, param_builder)
@app = CdnFonts.new(@app, theme: theme)
@app = CdnFonts.new(ctx, @app, theme: theme)
@app = LocalAssets.new(ctx, @app, extension)
@app = HotReload.new(ctx, @app, broadcast_hooks: broadcast_hooks, watcher: watcher, mode: mode,
script_injector: script_injector)
Expand Down
3 changes: 2 additions & 1 deletion packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/extension/dev_server/local_assets.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ module Theme
module Extension
class DevServer < ShopifyCLI::Theme::DevServer
class LocalAssets < ShopifyCLI::Theme::DevServer::LocalAssets
TAE_ASSET_REGEX = %r{(http:|https:)?//cdn\.shopify\.com/extensions/.+?/(assets/.+?\.(?:css|js))}
SUPPORTED_EXTENSIONS = [:jpg, :jpeg, :js, :css, :png, :svg].join("|")
TAE_ASSET_REGEX = %r{(http:|https:)?//cdn\.shopify\.com/extensions/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}

private

Expand Down
2 changes: 1 addition & 1 deletion packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/repl/auth_dev_server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def create_repl_theme

def middleware_stack
@app = proxy
@app = CdnFonts.new(app, theme: theme)
@app = CdnFonts.new(ctx, app, theme: theme)
@app = AuthMiddleware.new(app, proxy, repl) { WebServer.shutdown }
end

Expand Down
31 changes: 30 additions & 1 deletion packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ module ShopifyCLI
module Theme
class DevServer
class CdnFontsTest < Minitest::Test
def setup
super
Environment.stubs(:store).returns("my-test-shop.myshopify.com")
end

def test_replace_local_assistant_n4_font_in_reponse_body
original_html = <<~HTML
<html>
Expand Down Expand Up @@ -61,6 +66,28 @@ def test_replace_local_fonts_on_same_line
assert_equal(expected_html, serve(original_html).body)
end

def test_replace_shop_fonts_urls_in_reponse_body
original_html = <<~HTML
<html>
<head>
<link rel="preload" as="font" href="//my-test-shop.myshopify.com/cdn/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
<link rel="preload" as="font" href="http://my-test-shop.myshopify.com/cdn/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
<link rel="preload" as="font" href="https://my-test-shop.myshopify.com/cdn/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
</head>
</html>
HTML
expected_html = <<~HTML
<html>
<head>
<link rel="preload" as="font" href="/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
<link rel="preload" as="font" href="/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
<link rel="preload" as="font" href="/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
</head>
</html>
HTML
assert_equal(expected_html, serve(original_html).body)
end

def test_dont_replace_other_assets
original_html = <<~HTML
<html>
Expand Down Expand Up @@ -122,10 +149,11 @@ def test_replace_when_content_type_does_match_text
private

def serve(response_body = "", path: "/", content_type: "text/html")
ctx = TestHelpers::FakeContext.new(root: root)
app = lambda do |_env|
[200, { "Content-Type" => content_type }, [response_body]]
end
stack = CdnFonts.new(app, theme: theme)
stack = CdnFonts.new(ctx, app, theme: theme)
request = Rack::MockRequest.new(stack)
request.get(path)
end
Expand All @@ -136,6 +164,7 @@ def root

def theme
return @theme if @theme

@theme = Theme.new(nil, root: root)
@theme.stubs(shop: "my-test-shop.myshopify.com")
@theme
Expand Down
81 changes: 77 additions & 4 deletions packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/local_assets_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ module ShopifyCLI
module Theme
class DevServer
class LocalAssetsTest < Minitest::Test
def setup
super
Environment.stubs(:store).returns("my-test-shop.myshopify.com")
end

def test_replace_local_assets_in_reponse_body
original_html = <<~HTML
<html>
Expand Down Expand Up @@ -78,7 +83,7 @@ def test_serve_css_from_disk
assert_equal("text/css", response["Content-Type"])
assert_equal(
::File.read("#{ShopifyCLI::ROOT}/test/fixtures/theme/assets/theme.css"),
response.body
response.body,
)
end

Expand All @@ -87,7 +92,7 @@ def test_serve_js_from_disk
assert_equal("application/javascript", response["Content-Type"])
assert_equal(
::File.read("#{ShopifyCLI::ROOT}/test/fixtures/theme/assets/theme.css"),
response.body
response.body,
)
end

Expand All @@ -109,15 +114,83 @@ def test_404_on_missing_local_assets
assert_equal("Not found", response.body)
end

def test_replace_local_images_in_reponse_body
theme = stub("Theme", static_asset_paths: [
"assets/test-image.png",
"assets/test-image.png",
"assets/test-image.jpeg",
"assets/test-image.jpg",
"assets/test-vector.svg",
"assets/folha_de_estilo.css",
"assets/script.js",
"assets/static_object.json",
])

original_html = <<~HTML
<html>
<body>
<div data-src="//cdn.shopify.com/s/files/1/0000/1111/2222/t/333/assets/test-image.png?v=111111111111"></div>
<div data-src="//cdn.shopify.com/s/files/1/0000/1111/2222/t/333/assets/test-image.jpeg?v=111111111111"></div>
<div data-src="//cdn.shopify.com/s/files/1/0000/1111/2222/t/333/assets/test-image.jpg?v=111111111111"></div>
<div data-src="//cdn.shopify.com/s/files/1/0000/1111/2222/t/333/assets/test-vector.svg?v=111111111111"></div>
<div data-src="//cdn.shopify.com/s/files/1/0000/1111/2222/t/333/assets/folha_de_estilo.css?v=111111111111"></div>
<div data-src="//cdn.shopify.com/s/files/1/0000/1111/2222/t/333/assets/script.js?v=111111111111"></div>
</body>
</html>
HTML
expected_html = <<~HTML
<html>
<body>
<div data-src="/assets/test-image.png?v=111111111111"></div>
<div data-src="/assets/test-image.jpeg?v=111111111111"></div>
<div data-src="/assets/test-image.jpg?v=111111111111"></div>
<div data-src="/assets/test-vector.svg?v=111111111111"></div>
<div data-src="/assets/folha_de_estilo.css?v=111111111111"></div>
<div data-src="/assets/script.js?v=111111111111"></div>
</body>
</html>
HTML

assert_equal(expected_html, serve(original_html, theme_mock: theme).body)
end

def test_replace_shop_assets_urls_in_reponse_body
theme = stub("Theme", static_asset_paths: [
"assets/component-list-menu.css",
])

original_html = <<~HTML
<html>
<head>
<link rel="stylesheet" href="//my-test-shop.myshopify.com/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
<link rel="stylesheet" href="http://my-test-shop.myshopify.com/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
<link rel="stylesheet" href="https://my-test-shop.myshopify.com/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
</head>
</html>
HTML

expected_html = <<~HTML
<html>
<head>
<link rel="stylesheet" href="/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
<link rel="stylesheet" href="/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
<link rel="stylesheet" href="/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
</head>
</html>
HTML

assert_equal(expected_html, serve(original_html, theme_mock: theme).body)
end

private

def serve(response_body, path: "/")
def serve(response_body, path: "/", theme_mock: nil)
app = lambda do |_env|
[200, {}, [response_body]]
end
root = ShopifyCLI::ROOT + "/test/fixtures/theme"
ctx = TestHelpers::FakeContext.new(root: root)
theme = Theme.new(ctx, root: root)
theme = theme_mock || Theme.new(ctx, root: root)
stack = LocalAssets.new(ctx, app, theme)
request = Rack::MockRequest.new(stack)
request.get(path)
Expand Down
49 changes: 45 additions & 4 deletions .../cli-kit/assets/cli-ruby/test/shopify-cli/theme/extension/dev_server/local_assets_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# frozen_string_literal: true

require "test_helper"
require "shopify_cli/theme/extension/dev_server"
require "rack/mock"
Expand Down Expand Up @@ -78,7 +79,7 @@ def test_serve_css_from_disk
assert_equal("text/css", response["Content-Type"])
assert_equal(
::File.read("#{ShopifyCLI::ROOT}/test/fixtures/extension/assets/block1.css"),
response.body
response.body,
)
end

Expand All @@ -87,7 +88,7 @@ def test_serve_js_from_disk
assert_equal("application/javascript", response["Content-Type"])
assert_equal(
::File.read("#{ShopifyCLI::ROOT}/test/fixtures/extension/assets/block1.css"),
response.body
response.body,
)
end

Expand All @@ -109,15 +110,55 @@ def test_404_on_missing_local_assets
assert_equal("Not found", response.body)
end

def test_replace_local_images_in_reponse_body
extension = stub("Extension", static_asset_paths: [
"assets/test-image.png",
"assets/test-image.png",
"assets/test-image.jpeg",
"assets/test-image.jpg",
"assets/test-vector.svg",
"assets/folha_de_estilo.css",
"assets/script.js",
"assets/static_object.json",
])

original_html = <<~HTML
<html>
<body>
<div data-src="//cdn.shopify.com/extensions/s/files/1/0000/1111/2222/t/333/assets/test-image.png?v=111111111111"></div>
<div data-src="//cdn.shopify.com/extensions/s/files/1/0000/1111/2222/t/333/assets/test-image.jpeg?v=111111111111"></div>
<div data-src="//cdn.shopify.com/extensions/s/files/1/0000/1111/2222/t/333/assets/test-image.jpg?v=111111111111"></div>
<div data-src="//cdn.shopify.com/extensions/s/files/1/0000/1111/2222/t/333/assets/test-vector.svg?v=111111111111"></div>
<div data-src="//cdn.shopify.com/extensions/s/files/1/0000/1111/2222/t/333/assets/folha_de_estilo.css?v=111111111111"></div>
<div data-src="//cdn.shopify.com/extensions/s/files/1/0000/1111/2222/t/333/assets/script.js?v=111111111111"></div>
</body>
</html>
HTML
expected_html = <<~HTML
<html>
<body>
<div data-src="/assets/test-image.png?v=111111111111"></div>
<div data-src="/assets/test-image.jpeg?v=111111111111"></div>
<div data-src="/assets/test-image.jpg?v=111111111111"></div>
<div data-src="/assets/test-vector.svg?v=111111111111"></div>
<div data-src="/assets/folha_de_estilo.css?v=111111111111"></div>
<div data-src="/assets/script.js?v=111111111111"></div>
</body>
</html>
HTML

assert_equal(expected_html, serve(original_html, extension_mock: extension).body)
end

private

def serve(response_body, path: "/")
def serve(response_body, path: "/", extension_mock: nil)
app = lambda do |_env|
[200, {}, [response_body]]
end
root = ShopifyCLI::ROOT + "/test/fixtures/extension"
ctx = TestHelpers::FakeContext.new(root: root)
extension = AppExtension.new(ctx, root: root)
extension = extension_mock || AppExtension.new(ctx, root: root)
stack = LocalAssets.new(ctx, app, extension)
request = Rack::MockRequest.new(stack)
request.get(path)
Expand Down