From 04cbaa14758b2a36b0c1a09341d25c3ad16a71bf Mon Sep 17 00:00:00 2001 From: Ross Kaffenberger Date: Tue, 2 Jun 2020 23:27:43 -0400 Subject: [PATCH] Use ES module syntax for application.js.tt and docs This change swaps the CommonJS require() syntax in the Webpacker application.js pack template file and in documentation examples with ES module import syntax. Benefits of this change include: Provides continuity with the larger frontend community: Arguably, one of the main draws in adopting Webpacker is its integration with Babel to support ES module syntax. For a fresh Rails install with Webpacker, the application.js file will be the first impression most Rails developers have with webpack and Webpacker. Most of the recent documentation and examples they will find online for using other libraries will be based on ES module syntax. Reduces confusion: Developers commonly add ES imports to their application.js pack, typically by following online examples, which means mixing require() and import statements in a single file. This leads to confusion and unnecessary friction about differences between require() and import. Embraces browser-friendliness: The ES module syntax forward-looking and is meant to be supported in browsers. On the other hand, require() syntax is synchronous by design and not browser-supported as CommonJS originally was adopted in Node.js for server-side JavaScript. That webpack supports require() syntax is merely a convenience. Encourages best practices regarding optimization: webpack can statically analyze ES modules and "tree-shake", i.e., strip out unused exports from the final build (given certain conditions are met, including `sideEffects: false` designation in package.json). --- actionview/app/assets/javascripts/README.md | 3 ++- activestorage/README.md | 3 ++- guides/source/active_storage_overview.md | 3 ++- .../app/javascript/packs/application.js.tt | 15 +++++++++++---- railties/test/application/rake_test.rb | 2 +- .../test/generators/shared_generator_tests.rb | 5 +++-- 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/actionview/app/assets/javascripts/README.md b/actionview/app/assets/javascripts/README.md index c9282ba76f..3c0c8b0fd5 100644 --- a/actionview/app/assets/javascripts/README.md +++ b/actionview/app/assets/javascripts/README.md @@ -40,7 +40,8 @@ In a conventional Rails application that uses the asset pipeline, require `rails If you're using the Webpacker gem or some other JavaScript bundler, add the following to your main JS file: ```javascript -require("@rails/ujs").start() +import Rails from "@rails/ujs" +Rails.start() ``` ## How to run tests diff --git a/activestorage/README.md b/activestorage/README.md index f8f262da0a..6fe95d9382 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -151,7 +151,8 @@ Active Storage, with its included JavaScript library, supports uploading directl ``` Using the npm package: ```js - require("@rails/activestorage").start() + import * as ActiveStorage from "@rails/activestorage" + ActiveStorage.start() ``` 2. Annotate file inputs with the direct upload URL. diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index 6c0af950e7..c36f061867 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -578,7 +578,8 @@ directly from the client to the cloud. Using the npm package: ```js - require("@rails/activestorage").start() + import * as ActiveStorage from "@rails/activestorage" + ActiveStorage.start() ``` 2. Annotate file inputs with the direct upload URL. diff --git a/railties/lib/rails/generators/rails/app/templates/app/javascript/packs/application.js.tt b/railties/lib/rails/generators/rails/app/templates/app/javascript/packs/application.js.tt index e67e742263..f798b0bafb 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/javascript/packs/application.js.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/javascript/packs/application.js.tt @@ -3,17 +3,24 @@ // a relevant structure within app/javascript and only use these pack files to reference // that code so it'll be compiled. -require("@rails/ujs").start() +import Rails from "@rails/ujs" <%- unless options[:skip_turbolinks] -%> -require("turbolinks").start() +import Turbolinks from "turbolinks" <%- end -%> <%- unless skip_active_storage? -%> -require("@rails/activestorage").start() +import * as ActiveStorage from "@rails/activestorage" <%- end -%> <%- unless options[:skip_action_cable] -%> -require("channels") +import "channels" <%- end -%> +Rails.start() +<%- unless options[:skip_turbolinks] -%> +Turbolinks.start() +<%- end -%> +<%- unless skip_active_storage? -%> +ActiveStorage.start() +<%- end -%> // Uncomment to copy all static images under ../images to the output folder and reference // them with the image_pack_tag helper in views (e.g <%%= image_pack_tag 'rails.png' %>) diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index dfdd91bd92..2865c58334 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -157,7 +157,7 @@ module ApplicationTests end def test_code_statistics_sanity - assert_match "Code LOC: 29 Test LOC: 3 Code to Test Ratio: 1:0.1", + assert_match "Code LOC: 32 Test LOC: 3 Code to Test Ratio: 1:0.1", rails("stats") end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index c639b185a0..2a83f29f13 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -203,7 +203,8 @@ module SharedGeneratorTests unless generator_class.name == "Rails::Generators::PluginGenerator" assert_file "#{application_path}/app/javascript/packs/application.js" do |content| - assert_match(/^require\("@rails\/activestorage"\)\.start\(\)/, content) + assert_match(/^import \* as ActiveStorage from "@rails\/activestorage"/, content) + assert_match(/^ActiveStorage.start\(\)/, content) end end @@ -264,7 +265,7 @@ module SharedGeneratorTests assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']active_storage\/engine["']/ assert_file "#{application_path}/app/javascript/packs/application.js" do |content| - assert_no_match(/^require\("@rails\/activestorage"\)\.start\(\)/, content) + assert_no_match(/activestorage/i, content) end assert_file "#{application_path}/config/environments/development.rb" do |content|