Make Yarn the default, drop default vendor/asset directories (#27300)

11 files changed, 34 insertions, 111 deletions

diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index c445390cd3..0ba220104a 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -1,5 +1,4 @@
-*   Add Yarn support in new apps using --yarn option. This adds a yarn binstub, vendor/package.json,
-    and the settings needed to get npm modules integrated in new apps.
+*   Add Yarn support in new apps with a yarn binstub and vendor/package.json. Skippable via --skip-yarn option.
 
     *Liceth Ovalles*, *Guillermo Iguaran*, *DHH*
 
diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb
index 87f6f01750..aba68d6db2 100644
--- a/railties/lib/rails/generators/app_base.rb
+++ b/railties/lib/rails/generators/app_base.rb
@@ -33,8 +33,8 @@ module Rails
         class_option :javascript,         type: :string, aliases: "-j",
                                           desc: "Preconfigure for selected JavaScript library"
 
-        class_option :yarn,               type: :boolean, default: false,
-                                          desc: "Preconfigure for assets management with Yarn"
+        class_option :skip_yarn,          type: :boolean, default: false,
+                                          desc: "Don't use Yarn for managing JavaScript dependencies"
 
         class_option :skip_gemfile,       type: :boolean, default: false,
                                           desc: "Don't create a Gemfile"
@@ -414,55 +414,6 @@ module Rails
         bundle_command("install") if bundle_install?
       end
 
-      def run_yarn
-        if package_json_exist?
-          if yarn_path
-            say_status :run, "yarn install"
-            yarn_command("install")
-          else
-            say_status :warning, "yarn option passed but Yarn executable was not detected in the system.", :yellow
-            say_status :warning, "Download Yarn at https://yarnpkg.com/en/docs/install", :yellow
-          end
-        end
-      end
-
-      def package_json_exist?
-        File.exist?("vendor/package.json")
-      end
-
-      def yarn_path
-        commands = ["yarn"]
-
-        if Gem.win_platform?
-          ENV["PATHEXT"].split(File::PATH_SEPARATOR).each do |ext|
-            commands << commands[0] + ext
-          end
-        end
-
-        yarn_path = commands.find do |cmd|
-          paths = ENV["PATH"].split(File::PATH_SEPARATOR)
-
-          path = paths.find do |p|
-            full_path = File.expand_path(cmd, p)
-            File.executable?(full_path) && File.file?(full_path)
-          end
-
-          path && File.expand_path(cmd, path)
-        end
-
-        yarn_path
-      end
-
-      def yarn_command(command)
-        full_command = "#{yarn_path} #{command}"
-
-        if options[:quiet]
-          system(full_command, out: File::NULL)
-        else
-          system(full_command)
-        end
-      end
-
       def generate_spring_binstubs
         if bundle_install? && spring_install?
           bundle_command("exec spring binstub --all")
diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb
index 49d23d682a..20bc45fee8 100644
--- a/railties/lib/rails/generators/rails/app/app_generator.rb
+++ b/railties/lib/rails/generators/rails/app/app_generator.rb
@@ -53,10 +53,6 @@ module Rails
       template "gitignore", ".gitignore"
     end
 
-    def packagejson
-      template "package.json", "vendor/package.json"
-    end
-
     def app
       directory "app"
 
@@ -155,23 +151,12 @@ module Rails
     end
 
     def vendor
-      if options[:yarn]
-        empty_directory_with_keep_file "vendor"
-      else
-        vendor_javascripts
-        vendor_stylesheets
-      end
-    end
+      empty_directory_with_keep_file "vendor"
 
-    def vendor_javascripts
-      unless options[:skip_javascript]
-        empty_directory_with_keep_file "vendor/assets/javascripts"
+      unless options[:skip_yarn]
+        template "package.json", "vendor/package.json"
       end
     end
-
-    def vendor_stylesheets
-      empty_directory_with_keep_file "vendor/assets/stylesheets"
-    end
   end
 
   module Generators
@@ -213,9 +198,8 @@ module Rails
         build(:readme)
         build(:rakefile)
         build(:configru)
-        build(:packagejson) if options[:yarn]
-        build(:gitignore) unless options[:skip_git]
-        build(:gemfile)   unless options[:skip_gemfile]
+        build(:gitignore)   unless options[:skip_git]
+        build(:gemfile)     unless options[:skip_gemfile]
       end
 
       def create_app_files
@@ -276,6 +260,10 @@ module Rails
 
       def create_vendor_files
         build(:vendor)
+
+        if options[:skip_yarn]
+          remove_file "vendor/package.json"
+        end
       end
 
       def delete_app_assets_if_api_option
@@ -283,7 +271,6 @@ module Rails
           remove_dir "app/assets"
           remove_dir "lib/assets"
           remove_dir "tmp/cache/assets"
-          remove_dir "vendor/assets"
         end
       end
 
@@ -364,7 +351,7 @@ module Rails
       end
 
       public_task :apply_rails_template, :run_bundle
-      public_task :run_yarn, :generate_spring_binstubs
+      public_task :generate_spring_binstubs
 
       def run_after_bundle_callbacks
         @after_bundle_callbacks.each(&:call)
diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt
index 8db5b7e075..25870f19c8 100644
--- a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt
+++ b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt
@@ -1,8 +1,8 @@
 // This is a manifest file that'll be compiled into application.js, which will include all the files
 // listed below.
 //
-// Any JavaScript/Coffee file within this directory, lib/assets/javascripts, vendor/assets/javascripts,
-// or any plugin's vendor/assets/javascripts directory can be referenced here using a relative path.
+// Any JavaScript/Coffee file within this directory, lib/assets/javascripts, or any plugin's 
+// vendor/assets/javascripts directory can be referenced here using a relative path.
 //
 // It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the
 // compiled file. JavaScript code in this file should be added after the last require_* statement.
diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css
index 0ebd7fe829..865300bef9 100644
--- a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css
+++ b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css
@@ -2,8 +2,8 @@
  * This is a manifest file that'll be compiled into application.css, which will include all the files
  * listed below.
  *
- * Any CSS and SCSS file within this directory, lib/assets/stylesheets, vendor/assets/stylesheets,
- * or any plugin's vendor/assets/stylesheets directory can be referenced here using a relative path.
+ * Any CSS and SCSS file within this directory, lib/assets/stylesheets, or any plugin's 
+ * vendor/assets/stylesheets directory can be referenced here using a relative path.
  *
  * You're free to add application-wide styles to this file and they'll appear at the bottom of the
  * compiled file so the styles you add here take precedence over styles defined in any other CSS/SCSS
diff --git a/railties/lib/rails/generators/rails/app/templates/bin/setup.tt b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt
index 73b0732463..e4ba83a129 100644
--- a/railties/lib/rails/generators/rails/app/templates/bin/setup.tt
+++ b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt
@@ -16,8 +16,8 @@ chdir APP_ROOT do
   puts '== Installing dependencies =='
   system! 'gem install bundler --conservative'
   system('bundle check') || system!('bundle install')
-<% if options[:yarn] %>
-  system! 'bin/yarn'
+<% unless options[:skip_yarn] %>
+  system('bin/yarn') # Ignore failure from yarn not being installed
 
 <% end %>
 <% unless options.skip_active_record -%>
diff --git a/railties/lib/rails/generators/rails/app/templates/bin/update.tt b/railties/lib/rails/generators/rails/app/templates/bin/update.tt
index dcd80b6ce5..cc188315a8 100644
--- a/railties/lib/rails/generators/rails/app/templates/bin/update.tt
+++ b/railties/lib/rails/generators/rails/app/templates/bin/update.tt
@@ -16,8 +16,8 @@ chdir APP_ROOT do
   puts '== Installing dependencies =='
   system! 'gem install bundler --conservative'
   system('bundle check') || system!('bundle install')
-<% if options[:yarn] %>
-  system! 'bin/yarn'
+<% unless options[:skip_yarn] %>
+  system('bin/yarn') # Ignore failure from yarn not being installed
 
 <% end %>
 <% unless options.skip_active_record -%>
diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/assets.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/assets.rb.tt
index 29d9df6ad4..f5d03fb117 100644
--- a/railties/lib/rails/generators/rails/app/templates/config/initializers/assets.rb.tt
+++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/assets.rb.tt
@@ -5,7 +5,7 @@ Rails.application.config.assets.version = '1.0'
 
 # Add additional assets to the asset load path.
 # Rails.application.config.assets.paths << Emoji.images_path
-<%- if options[:yarn] -%>
+<%- unless options[:skip_yarn] -%>
 # Add Yarn node_modules folder to the asset load path.
 Rails.application.config.assets.paths << Rails.root.join('vendor/node_modules')
 <%- end -%>
diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore
index dee42ebb87..709b341387 100644
--- a/railties/lib/rails/generators/rails/app/templates/gitignore
+++ b/railties/lib/rails/generators/rails/app/templates/gitignore
@@ -21,7 +21,7 @@
 !/tmp/.keep
 <% end -%>
 
-<% if options[:yarn] -%>
+<% unless options[:skip_yarn] -%>
 /vendor/node_modules
 
 <% end -%>
diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb
index 7069a07bbe..dc2872e45e 100644
--- a/railties/test/generators/api_app_generator_test.rb
+++ b/railties/test/generators/api_app_generator_test.rb
@@ -108,7 +108,6 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase
          config/initializers/assets.rb
          config/initializers/cookies_serializer.rb
          lib/assets
-         vendor/assets
          test/helpers
          tmp/cache/assets
          public/404.html
diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb
index 15a923676d..d7c9ae5266 100644
--- a/railties/test/generators/app_generator_test.rb
+++ b/railties/test/generators/app_generator_test.rb
@@ -43,9 +43,6 @@ DEFAULT_APP_FILES = %w(
   test/mailers
   test/integration
   vendor
-  vendor/assets
-  vendor/assets/stylesheets
-  vendor/assets/javascripts
   tmp
   tmp/cache
   tmp/cache/assets
@@ -466,7 +463,6 @@ class AppGeneratorTest < Rails::Generators::TestCase
     run_generator [destination_root, "--skip-javascript"]
 
     assert_no_file "app/assets/javascripts"
-    assert_no_file "vendor/assets/javascripts"
 
     assert_file "app/views/layouts/application.html.erb" do |contents|
       assert_match(/stylesheet_link_tag\s+'application', media: 'all' %>/, contents)
@@ -492,12 +488,21 @@ class AppGeneratorTest < Rails::Generators::TestCase
     end
   end
 
-  def test_generator_if_yarn_option_is_given
-    run_generator([destination_root, "--yarn"])
+  def test_generator_for_yarn
+    run_generator([destination_root])
     assert_file "vendor/package.json", /dependencies/
     assert_file "config/initializers/assets.rb", /node_modules/
   end
 
+  def test_generator_for_yarn_skipped
+    run_generator([destination_root])
+    assert_no_file "vendor/package.json"
+
+    assert_file "config/environments/production.rb" do |content|
+      assert_no_match(/node_modules/, content)
+    end
+  end
+
   def test_inclusion_of_jbuilder
     run_generator
     assert_gem "jbuilder"
@@ -618,10 +623,6 @@ class AppGeneratorTest < Rails::Generators::TestCase
     assert_generates_with_bundler
   end
 
-  def test_generation_runs_yarn_install_with_yarn_option
-    assert_generates_with_yarn yarn: true
-  end
-
   def test_dev_option
     assert_generates_with_bundler dev: true
     rails_path = File.expand_path("../../..", Rails.root)
@@ -847,18 +848,4 @@ class AppGeneratorTest < Rails::Generators::TestCase
         quietly { generator.invoke_all }
       end
     end
-
-    def assert_generates_with_yarn(options = {})
-      generator([destination_root], options)
-
-      command_check = -> command do
-        @install_called ||= 0
-        @install_called += 1
-        assert_equal 1, @install_called, "install expected to be called once, but was called #{@install_called} times"
-      end
-
-      generator.stub :yarn_command, command_check do
-        quietly { generator.invoke_all }
-      end
-    end
 end