From b398d334725b45e9999fea6e62768f7f32eec3d6 Mon Sep 17 00:00:00 2001 From: Julien Poitrin Date: Fri, 17 Feb 2023 08:33:33 +0100 Subject: [PATCH] Add new changes from CLI 3 (`packages/cli-kit/assets/cli-ruby`) (#2729) ### WHY are these changes introduced? All fixes to embedded Ruby CLI should be added to both repos and new features only in the CLI3 repo. This is because we should give support to the old CLI repo until the sunset date on 31th May. ### WHAT is this pull request doing? Adds new changes from CLI 3 (`packages/cli-kit/assets/cli-ruby`) --- CHANGELOG.md | 2 ++ Gemfile.lock | 4 +-- lib/project_types/extension/commands/serve.rb | 10 ++++++ .../theme_app_extension.rb | 2 +- lib/shopify_cli/messages/messages.rb | 2 +- .../theme/dev_server/hot_reload.rb | 11 +++++-- .../dev_server/hot_reload/script_injector.rb | 2 +- lib/shopify_cli/theme/extension/dev_server.rb | 11 ++++--- lib/shopify_cli/theme/extension/host_theme.rb | 33 +++++++++++-------- shopify-cli.gemspec | 2 +- test/fixtures/shopify_schema.json | 4 +-- .../extension/commands/serve_test.rb | 3 ++ .../extension/features/argo_test.rb | 2 ++ .../theme/commands/serve_test.rb | 3 +- .../commands/populate/product_test.rb | 2 ++ .../theme/extension/dev_server_test.rb | 8 ++--- .../theme/syncer/uploader/bulk_test.rb | 1 + test/test_helper.rb | 2 +- .../lib/smart_properties/property.rb | 2 +- 19 files changed, 70 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abf6c7578a..60a97a8858 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ From version 2.6.0, the sections in this file adhere to the [keep a changelog](h ### Fixed * [#2721](https://github.com/Shopify/shopify-cli/pull/2721): Do not `replace_asset_urls` in font bodies +* [#2729](https://github.com/Shopify/shopify-cli/pull/2729): Do not inject hot-reload code into web-pixels-manager sandbox (from cli#1370) ### Added * [#2724](https://github.com/Shopify/shopify-cli/pull/2724): Introduce hidden `--overwrite-json` flag +* [#2729](https://github.com/Shopify/shopify-cli/pull/2729): Introduce hidden `--generate_tmp_theme` flag (from cli#1264) ## Version 2.34.0 - 2023-01-11 diff --git a/Gemfile.lock b/Gemfile.lock index 6d00c2318c..23af54ddd8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -118,7 +118,7 @@ GEM pry (~> 0.13.0) public_suffix (4.0.6) racc (1.6.2) - rack (2.2.6.2) + rack (2.2.3.1) rainbow (3.1.1) rake (13.0.6) rb-fsevent (0.11.1) @@ -165,7 +165,7 @@ PLATFORMS ruby DEPENDENCIES - bundler (~> 2.3.11) + bundler (>= 2.3.11) byebug colorize (~> 0.8.1) cucumber (~> 7.0) diff --git a/lib/project_types/extension/commands/serve.rb b/lib/project_types/extension/commands/serve.rb index 00f9c68245..bfd85231d9 100644 --- a/lib/project_types/extension/commands/serve.rb +++ b/lib/project_types/extension/commands/serve.rb @@ -20,6 +20,9 @@ class Serve < ExtensionCommand parser.on("-T", "--theme=NAME_OR_ID", "Theme ID or name of the theme app extension host theme.") do |theme| flags[:theme] = theme end + parser.on("--generate-tmp-theme", "Populate host theme, created by CLI 3, with assets") do |generate_tmp_theme| + flags[:generate_tmp_theme] = generate_tmp_theme + end parser.on("--api-key=API_KEY", "Connect your extension and app by inserting your app's API key") do |api_key| flags[:api_key] = api_key.gsub('"', "") end @@ -45,6 +48,7 @@ class RuntimeConfiguration property! :tunnel_requested, accepts: [true, false], reader: :tunnel_requested?, default: true property :port, accepts: (1...(2**16)) property :theme, accepts: String, default: nil + property :generate_tmp_theme, accepts: [true, false], reader: :generate_tmp_theme?, default: false property :api_key, accepts: String, default: nil property :api_secret, accepts: String, default: nil property :registration_id, accepts: String, default: nil @@ -60,6 +64,7 @@ def call(args, _command_name) resource_url: options.flags[:resource_url], port: options.flags[:port], theme: options.flags[:theme], + generate_tmp_theme: generate_tmp_theme?, api_key: options.flags[:api_key], api_secret: options.flags[:api_secret], registration_id: options.flags[:registration_id], @@ -103,6 +108,10 @@ def tunnel_requested? tunnel.nil? || !!tunnel end + def generate_tmp_theme? + options.flags[:generate_tmp_theme] == true + end + def find_available_port(runtime_configuration) return runtime_configuration unless runtime_configuration.port.nil? return runtime_configuration unless specification_handler.choose_port?(@ctx) @@ -137,6 +146,7 @@ def serve(runtime_configuration) tunnel_url: runtime_configuration.tunnel_url, port: runtime_configuration.port, theme: runtime_configuration.theme, + generate_tmp_theme: runtime_configuration.generate_tmp_theme?, api_key: runtime_configuration.api_key, api_secret: runtime_configuration.api_secret, registration_id: runtime_configuration.registration_id, diff --git a/lib/project_types/extension/models/specification_handlers/theme_app_extension.rb b/lib/project_types/extension/models/specification_handlers/theme_app_extension.rb index 10ff3a71a5..0546334404 100644 --- a/lib/project_types/extension/models/specification_handlers/theme_app_extension.rb +++ b/lib/project_types/extension/models/specification_handlers/theme_app_extension.rb @@ -78,7 +78,7 @@ def serve(**options) root = options[:context]&.root project = options[:project] properties = options - .slice(:port, :theme) + .slice(:port, :theme, :generate_tmp_theme) .compact .merge({ project: project, diff --git a/lib/shopify_cli/messages/messages.rb b/lib/shopify_cli/messages/messages.rb index c80c633094..5569328411 100644 --- a/lib/shopify_cli/messages/messages.rb +++ b/lib/shopify_cli/messages/messages.rb @@ -313,7 +313,7 @@ module Messages }, error_reporting: { unhandled_error: { - message: "{{x}} {{red:An unexpected error occured.}}", + message: "{{x}} {{red:An unexpected error occurred.}}", issue_message: "{{red:\tTo \e]8;;%s\e\\submit an issue\e]8;;\e\\"\ " include the stack trace.}}", stacktrace_message: "{{red:\tTo print the stack trace, add the environment variable %s.}}", diff --git a/lib/shopify_cli/theme/dev_server/hot_reload.rb b/lib/shopify_cli/theme/dev_server/hot_reload.rb index 2c236976d7..b31bd29c6e 100644 --- a/lib/shopify_cli/theme/dev_server/hot_reload.rb +++ b/lib/shopify_cli/theme/dev_server/hot_reload.rb @@ -16,12 +16,15 @@ def initialize(ctx, app, broadcast_hooks: [], script_injector: nil, watcher:, mo end def call(env) - if env["PATH_INFO"] == "/hot-reload" + path = env["PATH_INFO"] + if path == "/hot-reload" create_stream else status, headers, body = @app.call(env) - body = inject_hot_reload_javascript(body) if request_is_html?(headers) + if request_is_html?(headers) && leads_to_injectable_body?(path) + body = inject_hot_reload_javascript(body) + end [status, headers, body] end @@ -43,6 +46,10 @@ def request_is_html?(headers) headers["content-type"]&.start_with?("text/html") end + def leads_to_injectable_body?(path) + path !~ /web-pixels-manager.+sandbox/ + end + def inject_hot_reload_javascript(body) @script_injector&.inject(body: body, dir: __dir__, mode: @mode) end diff --git a/lib/shopify_cli/theme/dev_server/hot_reload/script_injector.rb b/lib/shopify_cli/theme/dev_server/hot_reload/script_injector.rb index 4e156d2342..98b718f1d8 100644 --- a/lib/shopify_cli/theme/dev_server/hot_reload/script_injector.rb +++ b/lib/shopify_cli/theme/dev_server/hot_reload/script_injector.rb @@ -25,7 +25,7 @@ def inject(body:, dir:, mode:) "", ].join("\n") - body = body.join.gsub("", "#{hot_reload_script}\n") + body = body.join.sub("", "#{hot_reload_script}\n") [body] end diff --git a/lib/shopify_cli/theme/extension/dev_server.rb b/lib/shopify_cli/theme/extension/dev_server.rb index f3ceb08631..32cc52abbc 100644 --- a/lib/shopify_cli/theme/extension/dev_server.rb +++ b/lib/shopify_cli/theme/extension/dev_server.rb @@ -26,12 +26,13 @@ class DevServer < ShopifyCLI::Theme::DevServer # Extensions ScriptInjector = ShopifyCLI::Theme::Extension::DevServer::HotReload::ScriptInjector - attr_accessor :project, :specification_handler + attr_accessor :project, :specification_handler, :generate_tmp_theme class << self - def start(ctx, root, port: 9292, theme: nil, project:, specification_handler:) + def start(ctx, root, port: 9292, theme: nil, generate_tmp_theme: false, project:, specification_handler:) instance.project = project instance.specification_handler = specification_handler + instance.generate_tmp_theme = generate_tmp_theme super(ctx, root, port: port, theme: theme) end @@ -66,8 +67,10 @@ def syncer def theme @theme ||= if theme_identifier - theme = ShopifyCLI::Theme::Theme.find_by_identifier(ctx, identifier: theme_identifier) - theme || ctx.abort(not_found_error_message) + theme = HostTheme.find_by_identifier(ctx, identifier: theme_identifier) + ctx.abort(not_found_error_message) unless theme + theme.generate_tmp_theme if generate_tmp_theme + theme else HostTheme.find_or_create!(ctx) end diff --git a/lib/shopify_cli/theme/extension/host_theme.rb b/lib/shopify_cli/theme/extension/host_theme.rb index 9142a801e1..1d698e9a7e 100644 --- a/lib/shopify_cli/theme/extension/host_theme.rb +++ b/lib/shopify_cli/theme/extension/host_theme.rb @@ -61,20 +61,9 @@ def self.find_or_create!(ctx) new(ctx, root: nil).ensure_exists! end - private - - def generate_host_theme_name - hostname = Socket.gethostname.split(".").shift - hash = SecureRandom.hex(3) - - theme_name = "App Ext. Host ()" - hostname_character_limit = API_NAME_LIMIT - theme_name.length - hash.length - 1 - identifier = encode_identifier("#{hash}-#{hostname[0, hostname_character_limit]}") - theme_name = "App Ext. Host (#{identifier})" - - ShopifyCLI::DB.set(host_theme_name: theme_name) - - theme_name + def self.find_by_identifier(ctx, root: nil, identifier:) + ShopifyCLI::DB.set(host_theme_id: identifier) + find(ctx, root: root) end def generate_tmp_theme @@ -98,6 +87,22 @@ def generate_tmp_theme end end end + + private + + def generate_host_theme_name + hostname = Socket.gethostname.split(".").shift + hash = SecureRandom.hex(3) + + theme_name = "App Ext. Host ()" + hostname_character_limit = API_NAME_LIMIT - theme_name.length - hash.length - 1 + identifier = encode_identifier("#{hash}-#{hostname[0, hostname_character_limit]}") + theme_name = "App Ext. Host (#{identifier})" + + ShopifyCLI::DB.set(host_theme_name: theme_name) + + theme_name + end end end end diff --git a/shopify-cli.gemspec b/shopify-cli.gemspec index 872b93383b..42b83ee717 100644 --- a/shopify-cli.gemspec +++ b/shopify-cli.gemspec @@ -35,7 +35,7 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib", "vendor"] spec.executables << "shopify" - spec.add_development_dependency("bundler", "~> 2.3.11") + spec.add_development_dependency("bundler", ">= 2.3.11") spec.add_development_dependency("rake", "~> 12.3", ">= 12.3.3") spec.add_development_dependency("minitest", "~> 5.0") diff --git a/test/fixtures/shopify_schema.json b/test/fixtures/shopify_schema.json index 1a9eb0f83d..99c9e83ac5 100644 --- a/test/fixtures/shopify_schema.json +++ b/test/fixtures/shopify_schema.json @@ -25325,7 +25325,7 @@ }, { "name": "createdAt", - "description": "When the activity event occured.", + "description": "When the activity event occurred.", "args": [], "type": { "kind": "NON_NULL", @@ -64374,4 +64374,4 @@ } } } -} \ No newline at end of file +} diff --git a/test/project_types/extension/commands/serve_test.rb b/test/project_types/extension/commands/serve_test.rb index a1dc41915d..42b079fc05 100644 --- a/test/project_types/extension/commands/serve_test.rb +++ b/test/project_types/extension/commands/serve_test.rb @@ -109,6 +109,7 @@ def test_resource_url_is_forwarded_to_specification_handler_if_one_is_provided tunnel_url: nil, port: 39351, theme: nil, + generate_tmp_theme: false, api_key: nil, api_secret: nil, registration_id: nil, @@ -131,6 +132,7 @@ def test_theme_is_forwarded_to_specification_handler_if_one_is_provided tunnel_url: nil, port: 39351, theme: theme, + generate_tmp_theme: false, api_key: nil, api_secret: nil, registration_id: nil, @@ -156,6 +158,7 @@ def test_auth_keys_are_forwarded_to_specification_handler_when_provided tunnel_url: nil, port: 39351, theme: nil, + generate_tmp_theme: false, api_key: api_key, api_secret: api_secret, registration_id: registration_id, diff --git a/test/project_types/extension/features/argo_test.rb b/test/project_types/extension/features/argo_test.rb index 89519e5735..403cb2f8ad 100644 --- a/test/project_types/extension/features/argo_test.rb +++ b/test/project_types/extension/features/argo_test.rb @@ -99,6 +99,7 @@ def test_config_returns_argo_renderer_package_version_if_it_exists_using_npm end def test_config_aborts_if_renderer_package_cannot_be_resolved + skip Base64.stubs(:strict_encode64).returns(@encoded_script) with_stubbed_script(@context, Argo::SCRIPT_PATH) do stub_run_yarn_install_and_run_yarn_run_script_methods @@ -113,6 +114,7 @@ def test_config_aborts_if_renderer_package_cannot_be_resolved end def test_config_aborts_with_error_when_argo_renderer_package_name_not_found + skip with_stubbed_script(@context, Features::Argo::SCRIPT_PATH) do stub_run_yarn_install_and_run_yarn_run_script_methods diff --git a/test/project_types/theme/commands/serve_test.rb b/test/project_types/theme/commands/serve_test.rb index 3e4d51f6e7..31e4887882 100644 --- a/test/project_types/theme/commands/serve_test.rb +++ b/test/project_types/theme/commands/serve_test.rb @@ -66,8 +66,7 @@ def test_can_specify_editor_sync end def test_can_specify_theme - ShopifyCLI::Theme::DevServer - .expects(:start) + ShopifyCLI::Theme::DevServer.expects(:start) .with(@ctx, @root, host: Theme::Command::Serve::DEFAULT_HTTP_HOST, theme: 1234) run_serve_command([@root]) do |command| diff --git a/test/shopify-cli/commands/populate/product_test.rb b/test/shopify-cli/commands/populate/product_test.rb index d1336ff0d3..2da628d4c0 100644 --- a/test/shopify-cli/commands/populate/product_test.rb +++ b/test/shopify-cli/commands/populate/product_test.rb @@ -8,6 +8,7 @@ class ProductTest < MiniTest::Test include TestHelpers::Schema def test_populate_calls_api_with_mutation + skip ShopifyCLI::Helpers::Haikunator.expects(:title).returns("fake product") ShopifyCLI::Helpers::Haikunator.expects(:title).returns("fake producttwo") ShopifyCLI::DB.expects(:exists?).with(:shop).returns(true).twice @@ -38,6 +39,7 @@ def test_populate_calls_api_with_mutation end def test_populate_if_no_shop + skip ShopifyCLI::DB.expects(:exists?).with(:shop).returns(false) ShopifyCLI::AdminAPI.expects(:query).never exception = assert_raises ShopifyCLI::Abort do diff --git a/test/shopify-cli/theme/extension/dev_server_test.rb b/test/shopify-cli/theme/extension/dev_server_test.rb index 459b959b27..5ea346b275 100644 --- a/test/shopify-cli/theme/extension/dev_server_test.rb +++ b/test/shopify-cli/theme/extension/dev_server_test.rb @@ -24,7 +24,7 @@ def test_middleware_stack end def test_theme_when_theme_does_not_exist - Theme + HostTheme .expects(:find_by_identifier) .with(ctx, identifier: theme.id) .returns(nil) @@ -37,7 +37,7 @@ def test_theme_when_theme_does_not_exist end def test_theme_with_valid_theme_id - Theme + HostTheme .expects(:find_by_identifier) .with(ctx, identifier: theme.id) .returns(theme) @@ -46,7 +46,7 @@ def test_theme_with_valid_theme_id end def test_theme_with_valid_theme_name - Theme + HostTheme .expects(:find_by_identifier) .with(ctx, identifier: theme.name) .returns(theme) @@ -55,7 +55,7 @@ def test_theme_with_valid_theme_name end def test_finds_or_creates_a_dev_theme_when_no_theme_specified - Theme + HostTheme .expects(:find_by_identifier).never HostTheme .expects(:find_or_create!) diff --git a/test/shopify-cli/theme/syncer/uploader/bulk_test.rb b/test/shopify-cli/theme/syncer/uploader/bulk_test.rb index defc4d7f98..842a659dc5 100644 --- a/test/shopify-cli/theme/syncer/uploader/bulk_test.rb +++ b/test/shopify-cli/theme/syncer/uploader/bulk_test.rb @@ -79,6 +79,7 @@ def test_batch_num_files_upper_bound_with_single_thread end def test_batch_num_files_upper_bound_with_multiple_threads + skip # flaky bulk = bulk_instance(pool_size: 2) expect_job_request(10, 100).twice diff --git a/test/test_helper.rb b/test/test_helper.rb index 46e2f6f217..ef707b4e84 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -20,4 +20,4 @@ c.stubbing_method_on_nil = :prevent end -Minitest::Reporters.use!([Minitest::Reporters::DefaultReporter.new(color: true)]) unless ENV["RM_INFO"] +Minitest::Reporters.use!([Minitest::Reporters::SpecReporter.new(print_at_bottom: true)]) unless ENV["RM_INFO"] diff --git a/vendor/deps/smart_properties/lib/smart_properties/property.rb b/vendor/deps/smart_properties/lib/smart_properties/property.rb index 6198e71870..7974b70efe 100644 --- a/vendor/deps/smart_properties/lib/smart_properties/property.rb +++ b/vendor/deps/smart_properties/lib/smart_properties/property.rb @@ -157,7 +157,7 @@ def null_object?(object) rescue NoMethodError => error # BasicObject does not respond to #nil? by default, so we need to double # check if somebody implemented it and it fails internally or if the - # error occured because the method is actually not present. In the former + # error occurred because the method is actually not present. In the former # case, we want to raise the exception because there is something wrong # with the implementation of object#nil?. In the latter case we treat the # object as truthy because we don't know better.