From 6e44e5c2b56da50ba52d89550bb00a768501ba22 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 26 Mar 2024 08:21:47 +1300 Subject: [PATCH] refactor: make manager checker a utility --- lib/install/template.rb | 14 ++-- lib/shakapacker/instance.rb | 1 - lib/shakapacker/manager_checker.rb | 54 ---------------- lib/shakapacker/runner.rb | 3 +- lib/shakapacker/utils/manager.rb | 58 +++++++++++++++++ lib/tasks/shakapacker/check_manager.rake | 4 +- spec/generator_specs/generator_spec.rb | 1 - spec/shakapacker/dev_server_runner_spec.rb | 6 +- ..._checker_spec.rb => utils_manager_spec.rb} | 64 ++++++++----------- spec/shakapacker/webpack_runner_spec.rb | 6 +- 10 files changed, 97 insertions(+), 114 deletions(-) delete mode 100644 lib/shakapacker/manager_checker.rb create mode 100644 lib/shakapacker/utils/manager.rb rename spec/shakapacker/{manager_checker_spec.rb => utils_manager_spec.rb} (63%) diff --git a/lib/install/template.rb b/lib/install/template.rb index 5c8c5c9e2..2cce17cfe 100644 --- a/lib/install/template.rb +++ b/lib/install/template.rb @@ -1,4 +1,5 @@ require "shakapacker/utils/misc" +require "shakapacker/utils/manager" require "shakapacker/utils/version_syntax_converter" # Install Shakapacker @@ -59,13 +60,10 @@ def package_json babel["presets"] ||= [] babel["presets"].push("./node_modules/shakapacker/package/babel/preset.js") - package_manager = pj.fetch("packageManager", nil) - - if package_manager.nil? - manager_checker = Shakapacker::ManagerChecker.new - - package_manager = "#{manager_checker.guess_manager}@#{manager_checker.guess_manager_version}" - end + package_manager = pj.fetch( + "packageManager", + "#{Shakapacker::Utils::Manager.guess_binary}@#{Shakapacker::Utils::Manager.guess_version}" + ) { "name" => "app", @@ -79,7 +77,7 @@ def package_json }.merge(pj) end -Shakapacker::ManagerChecker.new.warn_unless_package_manager_is_obvious! +Shakapacker::Utils::Manager.warn_unless_package_manager_is_obvious! # Ensure there is `system!("bin/yarn")` command in `./bin/setup` file if (setup_path = Rails.root.join("bin/setup")).exist? diff --git a/lib/shakapacker/instance.rb b/lib/shakapacker/instance.rb index 5b06d0ea8..735b966dc 100644 --- a/lib/shakapacker/instance.rb +++ b/lib/shakapacker/instance.rb @@ -1,5 +1,4 @@ require "pathname" -require "shakapacker/manager_checker" class Shakapacker::Instance cattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) } diff --git a/lib/shakapacker/manager_checker.rb b/lib/shakapacker/manager_checker.rb deleted file mode 100644 index 28deb4968..000000000 --- a/lib/shakapacker/manager_checker.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true -require "shakapacker/version" -require "package_json" - -module Shakapacker - class ManagerChecker - class Error < StandardError; end - - MANAGER_LOCKS = { - bun: "bun.lockb", - npm: "package-lock.json", - pnpm: "pnpm-lock.yaml", - yarn: "yarn.lock" - } - - # Emits a warning if it's not obvious what package manager to use - def warn_unless_package_manager_is_obvious! - return if package_manager_set? - - guessed_manager = guess_manager - - return if guess_manager == :npm - - Shakapacker.puts_deprecation_message(<<~MSG) - You have not got "packageManager" set in your package.json meaning that Shakapacker will use npm - but you've got a #{MANAGER_LOCKS[guessed_manager]} file meaning you probably want - to be using #{guessed_manager} instead. - - To make this happen, set "packageManager" in your package.json to #{guessed_manager}@#{guess_manager_version} - MSG - end - - def package_manager_set? - !PackageJson.read.fetch("packageManager", nil).nil? - end - - def guess_manager_version - require "open3" - - command = "#{guess_manager} --version" - stdout, stderr, status = Open3.capture3(command) - - unless status.success? - raise Error, "#{command} failed with exit code #{status.exitstatus}: #{stderr}" - end - - stdout.chomp - end - - def guess_manager - MANAGER_LOCKS.find { |_, lock| File.exist?(lock) }&.first || :npm - end - end -end diff --git a/lib/shakapacker/runner.rb b/lib/shakapacker/runner.rb index 02f92899b..048a4e1bc 100644 --- a/lib/shakapacker/runner.rb +++ b/lib/shakapacker/runner.rb @@ -1,4 +1,5 @@ require "shakapacker/utils/misc" +require "shakapacker/utils/manager" module Shakapacker class Runner @@ -20,7 +21,7 @@ def initialize(argv) exit! end - ManagerChecker.new.warn_unless_package_manager_is_obvious! + Shakapacker::Utils::Manager.warn_unless_package_manager_is_obvious! end def package_json diff --git a/lib/shakapacker/utils/manager.rb b/lib/shakapacker/utils/manager.rb new file mode 100644 index 000000000..833790460 --- /dev/null +++ b/lib/shakapacker/utils/manager.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true +# +require "package_json" + +module Shakapacker + module Utils + class Manager + class Error < StandardError; end + + MANAGER_LOCKS = { + "bun" => "bun.lockb", + "npm" => "package-lock.json", + "pnpm" => "pnpm-lock.yaml", + "yarn" => "yarn.lock" + } + + # Emits a warning if it's not obvious what package manager to use + def self.warn_unless_package_manager_is_obvious! + return unless PackageJson.read.fetch("packageManager", nil).nil? + + guessed_binary = guess_binary + + return if guessed_binary == "npm" + + Shakapacker.puts_deprecation_message(<<~MSG) + You have not got "packageManager" set in your package.json meaning that Shakapacker will use npm + but you've got a #{MANAGER_LOCKS[guessed_binary]} file meaning you probably want + to be using #{guessed_binary} instead. + + To make this happen, set "packageManager" in your package.json to #{guessed_binary}@#{guess_version} + MSG + end + + # Guesses the binary of the package manager to use based on what lockfiles exist + # + # @return [String] + def self.guess_binary + MANAGER_LOCKS.find { |_, lock| File.exist?(lock) }&.first || "npm" + end + + # Guesses the version of the package manager to use by calling ` --version` + # + # @return [String] + def self.guess_version + require "open3" + + command = "#{guess_binary} --version" + stdout, stderr, status = Open3.capture3(command) + + unless status.success? + raise Error, "#{command} failed with exit code #{status.exitstatus}: #{stderr}" + end + + stdout.chomp + end + end + end +end diff --git a/lib/tasks/shakapacker/check_manager.rake b/lib/tasks/shakapacker/check_manager.rake index 72fde8b9b..4112ab9eb 100644 --- a/lib/tasks/shakapacker/check_manager.rake +++ b/lib/tasks/shakapacker/check_manager.rake @@ -1,10 +1,10 @@ require "shakapacker/utils/misc" -require "shakapacker/manager_checker" +require "shakapacker/utils/manager" namespace :shakapacker do desc "Verifies if the expected JS package manager is installed" task :check_manager do - Shakapacker::ManagerChecker.new.warn_unless_package_manager_is_obvious! + Shakapacker::Utils::Manager.warn_unless_package_manager_is_obvious! require "package_json" diff --git a/spec/generator_specs/generator_spec.rb b/spec/generator_specs/generator_spec.rb index 16776d51e..ed4ed0fb1 100644 --- a/spec/generator_specs/generator_spec.rb +++ b/spec/generator_specs/generator_spec.rb @@ -3,7 +3,6 @@ require "json" require "shakapacker/utils/misc" require "shakapacker/utils/version_syntax_converter" -require "shakapacker/manager_checker" require "package_json" GEM_ROOT = Pathname.new(File.expand_path("../../..", __FILE__)) diff --git a/spec/shakapacker/dev_server_runner_spec.rb b/spec/shakapacker/dev_server_runner_spec.rb index 22aa0e714..6a85ee1eb 100644 --- a/spec/shakapacker/dev_server_runner_spec.rb +++ b/spec/shakapacker/dev_server_runner_spec.rb @@ -32,11 +32,9 @@ PackageJson.read.merge! { { "packageManager" => "#{manager_name}@#{manager_version}" } } - allow(Shakapacker::ManagerChecker).to receive(:new).and_return(fake_manager_checker) - allow(fake_manager_checker).to receive(:warn_unless_package_manager_is_obvious!) + allow(Shakapacker::Utils::Manager).to receive(:warn_unless_package_manager_is_obvious!) end - let(:fake_manager_checker) { instance_double(Shakapacker::ManagerChecker).as_null_object } let(:package_json) { PackageJson.read(test_app_path) } require "package_json" @@ -134,7 +132,7 @@ def verify_command(cmd, argv: [], env: Shakapacker::Compiler.env) klass.run(argv) expect(Kernel).to have_received(:exec).with(env, *cmd) - expect(fake_manager_checker).to have_received(:warn_unless_package_manager_is_obvious!) + expect(Shakapacker::Utils::Manager).to have_received(:warn_unless_package_manager_is_obvious!) end end end diff --git a/spec/shakapacker/manager_checker_spec.rb b/spec/shakapacker/utils_manager_spec.rb similarity index 63% rename from spec/shakapacker/manager_checker_spec.rb rename to spec/shakapacker/utils_manager_spec.rb index 2671f5ba6..2ba453880 100644 --- a/spec/shakapacker/manager_checker_spec.rb +++ b/spec/shakapacker/utils_manager_spec.rb @@ -1,5 +1,5 @@ require_relative "spec_helper_initializer" -require "shakapacker/version" +require "shakapacker/utils/manager" Struct.new("Status", :exit_code) do def success? @@ -11,12 +11,12 @@ def exitstatus end end -describe "ManagerChecker" do +describe "Shakapacker::Utils::Manager" do around do |example| within_temp_directory { example.run } end - describe "#warn_unless_package_manager_is_obvious!" do + describe "~warn_unless_package_manager_is_obvious!" do before do allow(Shakapacker).to receive(:puts_deprecation_message) end @@ -27,7 +27,7 @@ def exitstatus end it "does nothing" do - Shakapacker::ManagerChecker.new.warn_unless_package_manager_is_obvious! + Shakapacker::Utils::Manager.warn_unless_package_manager_is_obvious! expect(Shakapacker).not_to have_received(:puts_deprecation_message) end @@ -38,13 +38,13 @@ def exitstatus File.write("package.json", {}.to_json) FileUtils.touch("package-lock.json") - Shakapacker::ManagerChecker.new.warn_unless_package_manager_is_obvious! + Shakapacker::Utils::Manager.warn_unless_package_manager_is_obvious! expect(Shakapacker).not_to have_received(:puts_deprecation_message) end end - Shakapacker::ManagerChecker::MANAGER_LOCKS.reject { |manager| manager == :npm }.each do |manager, lock| + Shakapacker::Utils::Manager::MANAGER_LOCKS.reject { |manager| manager == "npm" }.each do |manager, lock| context "when there is a #{lock}" do before do allow(Open3).to receive(:capture3).and_return(["1.2.3\n", "", Struct::Status.new(0)]) @@ -54,7 +54,7 @@ def exitstatus File.write("package.json", {}.to_json) FileUtils.touch(lock) - Shakapacker::ManagerChecker.new.warn_unless_package_manager_is_obvious! + Shakapacker::Utils::Manager.warn_unless_package_manager_is_obvious! expect(Shakapacker).to have_received(:puts_deprecation_message).with(<<~MSG) You have not got "packageManager" set in your package.json meaning that Shakapacker will use npm @@ -68,31 +68,35 @@ def exitstatus end end - describe "#package_manager_set?" do - it "returns true when the 'packageManager' property is set" do - File.write("package.json", { "packageManager" => "npm" }.to_json) + describe "~guess_binary" do + Shakapacker::Utils::Manager::MANAGER_LOCKS.each do |manager, lock| + context "when a #{lock} exists" do + before { FileUtils.touch(lock) } - expect(Shakapacker::ManagerChecker.new.package_manager_set?).to be true + it "guesses #{manager}" do + expect(Shakapacker::Utils::Manager.guess_binary).to eq manager + end + end end - it "returns false when the 'packageManager' property is not set" do - File.write("package.json", {}.to_json) - - expect(Shakapacker::ManagerChecker.new.package_manager_set?).to be false + context "when there is no lockfile" do + it "returns npm" do + expect(Shakapacker::Utils::Manager.guess_binary).to eq "npm" + end end end - describe "#guess_manager_version" do + describe "~guess_version" do before do allow(Open3).to receive(:capture3).and_return(["1.2.3\n", "", Struct::Status.new(0)]) end - Shakapacker::ManagerChecker::MANAGER_LOCKS.each do |manager, lock| + Shakapacker::Utils::Manager::MANAGER_LOCKS.each do |manager, lock| context "when a #{lock} exists" do before { FileUtils.touch(lock) } it "calls #{manager} with --version" do - Shakapacker::ManagerChecker.new.guess_manager_version + Shakapacker::Utils::Manager.guess_version expect(Open3).to have_received(:capture3).with("#{manager} --version") end @@ -102,7 +106,7 @@ def exitstatus it "returns the output without a trailing newline" do FileUtils.touch("package-lock.json") - expect(Shakapacker::ManagerChecker.new.guess_manager_version).to eq("1.2.3") + expect(Shakapacker::Utils::Manager.guess_version).to eq("1.2.3") end context "when the command errors" do @@ -113,29 +117,11 @@ def exitstatus it "raises an error" do FileUtils.touch("package-lock.json") - expect { Shakapacker::ManagerChecker.new.guess_manager_version }.to raise_error( - Shakapacker::ManagerChecker::Error, + expect { Shakapacker::Utils::Manager.guess_version }.to raise_error( + Shakapacker::Utils::Manager::Error, "npm --version failed with exit code 1: oh noes!" ) end end end - - describe "#guess_manager" do - Shakapacker::ManagerChecker::MANAGER_LOCKS.each do |manager, lock| - context "when a #{lock} exists" do - before { FileUtils.touch(lock) } - - it "guesses #{manager}" do - expect(Shakapacker::ManagerChecker.new.guess_manager).to eq manager - end - end - end - - context "when there is no lockfile" do - it "returns npm" do - expect(Shakapacker::ManagerChecker.new.guess_manager).to eq :npm - end - end - end end diff --git a/spec/shakapacker/webpack_runner_spec.rb b/spec/shakapacker/webpack_runner_spec.rb index e9086bef4..100c6a087 100644 --- a/spec/shakapacker/webpack_runner_spec.rb +++ b/spec/shakapacker/webpack_runner_spec.rb @@ -30,11 +30,9 @@ PackageJson.read.merge! { { "packageManager" => "#{manager_name}@#{manager_version}" } } - allow(Shakapacker::ManagerChecker).to receive(:new).and_return(fake_manager_checker) - allow(fake_manager_checker).to receive(:warn_unless_package_manager_is_obvious!) + allow(Shakapacker::Utils::Manager).to receive(:warn_unless_package_manager_is_obvious!) end - let(:fake_manager_checker) { instance_double(Shakapacker::ManagerChecker).as_null_object } let(:package_json) { PackageJson.read(test_app_path) } require "package_json" @@ -74,7 +72,7 @@ def verify_command(cmd, argv: []) klass.run(argv) expect(Kernel).to have_received(:exec).with(Shakapacker::Compiler.env, *cmd) - expect(fake_manager_checker).to have_received(:warn_unless_package_manager_is_obvious!) + expect(Shakapacker::Utils::Manager).to have_received(:warn_unless_package_manager_is_obvious!) end end end