From e0975f5a1f339049c249c03cf4e025713b2e0b2e Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Fri, 26 May 2017 15:44:29 -1000 Subject: [PATCH] Better error messages and API tweaks (#12) * Added better error messages for missing files. * Added API to check if the manifest exists. * Added CONTRIBUTING.md doc --- .travis.yml | 3 +- CHANGELOG.md | 3 ++ CONTRIBUTING.md | 70 +++++++++++++++++++++++++++++ README.md | 4 +- Rakefile | 17 ++++++- lib/webpacker_lite.rb | 6 +-- lib/webpacker_lite/configuration.rb | 11 ++--- lib/webpacker_lite/env.rb | 2 +- lib/webpacker_lite/file_loader.rb | 8 ++-- lib/webpacker_lite/manifest.rb | 53 +++++++++++++++++----- test/manifest_test.rb | 8 ++-- 11 files changed, 153 insertions(+), 32 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/.travis.yml b/.travis.yml index 34a90be..583a37d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,5 +10,4 @@ cache: install: - bundle install script: - - bundle exec rubocop - - bundle exec rake test + - bundle exec rake diff --git a/CHANGELOG.md b/CHANGELOG.md index f650021..dccdc8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ Contributors: please follow the recommendations outlined at [keepachangelog.com] ## [Unreleased] *Please add entries here for your pull requests.* +### Changed +* Added better error messages for missing files. +* Added api to check if the manifest exists ## [2.0.0] - 2017-05-23 All in [#9](https://github.com/shakacode/webpacker_lite/pull/9) by [justin808](https://github.com/justin808) with help from [conturbo](https://github.com/conturbo) on the tests. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..5623c68 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,70 @@ +# Tips for Contributors + +## Summary + +For non-doc fixes: + +* Provide changelog entry in the [unreleased section of the CHANGELOG.md](https://github.com/shakacode/react_on_rails/blob/master/CHANGELOG.md#unreleased). +* Ensure CI passes and that you added a test that passes with the fix and fails without the fix. +* Optionally, squash all commits down to one with a nice commit message *ONLY* once final review is given. Make sure this single commit is rebased on top of master. +* Please address all code review comments. +* Ensure that docs are updated accordingly if a feature is added. + +## Commit Messages + +From [How to Write a Git Commit Message](http://chris.beams.io/posts/git-commit/) + +#### The seven rules of a great git commit message +> Keep in mind: This has all been said before. + +1. Separate subject from body with a blank line +1. Limit the subject line to 50 characters +1. Capitalize the subject line +1. Do not end the subject line with a period +1. Use the imperative mood in the subject line +1. Wrap the body at 72 characters +1. Use the body to explain what and why vs. how + + +## To run: + +### Tests +``` +rake test +``` + +### Linting + +``` +rake rubocop +``` + +or to autofix: + +``` +bundle exec rubocop -a +``` + +### All ci + +```sh +rake +``` + +# Configuring to test changes with your app + +Use the relative path syntax in your gemfile. + +```ruby +gem "webpacker_lite", path: "../../../webpacker_lite" +``` + +# Advice for Project Maintainers and Contributors + +What do project maintainers do? What sort of work is involved? [sstephenson](https://github.com/sstephenson) wrote in the [turbolinks](https://github.com/turbolinks/turbolinks) repo: + +> [Why this is not still fully merged?](https://github.com/turbolinks/turbolinks/pull/124#issuecomment-239826060) + +# Releasing + + diff --git a/README.md b/README.md index 9a1aee7..6d81c22 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ This example config shows how we use different output directories for the webpac default: &default manifest: manifest.json # Used in your webpack configuration. Must be created in the - # webpack_public_output_dir folder + # webpack_public_output_dir folder. development: <<: *default @@ -141,6 +141,7 @@ production: ## Other Helpers: Getting the asset path The `asset_pack_path` helper provides the path of any given asset that's been compiled by webpack. +Note, the real file path is the subdirectory of the public. For example, if you want to create a `` or `` for an asset used in your pack code you can reference them like this in your view, @@ -148,6 +149,7 @@ for an asset used in your pack code you can reference them like this in your vie ```erb <% # => %> +<% # real file path "public/webpack/calendar.png" /> %> ``` ## Webpack Helper diff --git a/Rakefile b/Rakefile index 249c285..9b55832 100644 --- a/Rakefile +++ b/Rakefile @@ -2,6 +2,16 @@ require "bundler/gem_tasks" require "rake/testtask" +# Executes a string or an array of strings in a shell in the given directory +def sh_in_dir(dir, shell_commands) + shell_commands = [shell_commands] if shell_commands.is_a?(String) + shell_commands.each { |shell_command| sh %(cd #{dir} && #{shell_command.strip}) } +end + +def gem_root + File.expand_path("../.", __FILE__) +end + Rake::TestTask.new(:test) do |t| t.libs << "test" t.libs << "lib" @@ -9,4 +19,9 @@ Rake::TestTask.new(:test) do |t| t.verbose = true end -task default: :test +desc "Run Rubocop as shell" +task :rubocop do + sh_in_dir(gem_root, "bundle exec rubocop .") +end + +task default: [:test, :rubocop] diff --git a/lib/webpacker_lite.rb b/lib/webpacker_lite.rb index cb6da63..6609dc0 100644 --- a/lib/webpacker_lite.rb +++ b/lib/webpacker_lite.rb @@ -1,8 +1,8 @@ module WebpackerLite def self.bootstrap - WebpackerLite::Env.load - WebpackerLite::Configuration.load - WebpackerLite::Manifest.load + WebpackerLite::Env.load_instance + WebpackerLite::Configuration.load_instance + WebpackerLite::Manifest.load_instance end def env diff --git a/lib/webpacker_lite/configuration.rb b/lib/webpacker_lite/configuration.rb index 5041e17..571e82a 100644 --- a/lib/webpacker_lite/configuration.rb +++ b/lib/webpacker_lite/configuration.rb @@ -3,6 +3,8 @@ require "webpacker_lite/env" class WebpackerLite::Configuration < WebpackerLite::FileLoader + RAILS_WEB_PUBLIC = "public" + class << self def manifest_path Rails.root.join(webpack_public_output_dir, @@ -10,8 +12,7 @@ def manifest_path end def webpack_public_output_dir - Rails.root.join( - File.join("public", configuration.fetch(:webpack_public_output_dir, "webpack"))) + Rails.root.join(RAILS_WEB_PUBLIC, configuration.fetch(:webpack_public_output_dir, "webpack")) end def base_path @@ -37,8 +38,8 @@ def base_url end def configuration - load if WebpackerLite::Env.development? - raise WebpackerLite::FileLoader::FileLoaderError.new("WebpackerLite::Configuration.load must be called first") unless instance + load_instance if WebpackerLite::Env.development? + raise WebpackerLite::FileLoader::FileLoaderError.new("WebpackerLite::Configuration.load_instance must be called first") unless instance instance.data end @@ -48,7 +49,7 @@ def file_path end private - def load + def load_data return super unless File.exist?(@path) HashWithIndifferentAccess.new(YAML.load(File.read(@path))[WebpackerLite::Env.current]) end diff --git a/lib/webpacker_lite/env.rb b/lib/webpacker_lite/env.rb index 44f632a..d7f029e 100644 --- a/lib/webpacker_lite/env.rb +++ b/lib/webpacker_lite/env.rb @@ -25,7 +25,7 @@ def file_path end private - def load + def load_data environments = File.exist?(@path) ? YAML.load(File.read(@path)).keys : [].freeze return ENV["NODE_ENV"] if environments.include?(ENV["NODE_ENV"]) return Rails.env if environments.include?(Rails.env) diff --git a/lib/webpacker_lite/file_loader.rb b/lib/webpacker_lite/file_loader.rb index 634f269..60fba95 100644 --- a/lib/webpacker_lite/file_loader.rb +++ b/lib/webpacker_lite/file_loader.rb @@ -7,22 +7,22 @@ class FileLoaderError < StandardError; end attr_accessor :data class << self - def load(path = file_path) + def load_instance(path = file_path) self.instance = new(path) end def file_path - raise "Subclass of WebpackerLite::FileLoader should override this method" + raise FileLoaderError.new("Subclass of WebpackerLite::FileLoader should override this method") end end private def initialize(path) @path = path - @data = load + @data = load_data end - def load + def load_data {}.freeze end end diff --git a/lib/webpacker_lite/manifest.rb b/lib/webpacker_lite/manifest.rb index adce74b..64bf85b 100644 --- a/lib/webpacker_lite/manifest.rb +++ b/lib/webpacker_lite/manifest.rb @@ -11,30 +11,61 @@ class WebpackerLite::Manifest < WebpackerLite::FileLoader class << self - def file_path - WebpackerLite::Configuration.manifest_path + # Helper method to determine if the manifest file exists. + def exist? + path_object = WebpackerLite::Configuration.manifest_path + path_object.exist? end - def lookup(name) - load if WebpackerLite::Env.development? || instance.data.empty? - raise WebpackerLite::FileLoader::FileLoaderError.new("WebpackerLite::Manifest.load must be called first") unless instance - instance.data[name.to_s] || missing_file_error(name) + def file_path + WebpackerLite::Configuration.manifest_path end - def missing_file_error(name) + def missing_file_from_manifest_error(bundle_name) msg = <<-MSG - WebpackerLite can't find #{name} in your manifest #{file_path}. Possible causes: - 1. You are hot reloading - 2. Webpack is not running + WebpackerLite can't find #{bundle_name} in your manifest #{file_path}. Possible causes: + 1. You are hot reloading. + 2. Webpack has not re-run to reflect updates. 3. You have misconfigured WebpackerLite's config/webpacker_lite.yml file. 4. Your Webpack configuration is not creating a manifest. MSG raise(WebpackerLite::FileLoader::NotFoundError.new(msg)) end + + # Same as lookup, but raises an error. + def lookup!(name) + lookup(name) || missing_file_from_manifest_error(name) + end + + # Find the real file name from the manifest key. + def lookup(name) + instance.confirm_manifest_exists + + load_instance if WebpackerLite::Env.development? || instance.data.empty? + raise WebpackerLite::FileLoader::FileLoaderError.new("WebpackerLite::Manifest.load must be called first") unless instance + instance.data[name.to_s] + end + end + + def confirm_manifest_exists + raise missing_manifest_file_error(@path) unless File.exist?(@path) end private - def load + + def missing_manifest_file_error(path_object) + msg = <<-MSG + + WebpackerLite can't find the manifest file: #{path_object} + Possible causes: + 1. You have not invoked webpack. + 2. You have misconfigured WebpackerLite's config/webpacker_lite.yml file. + 3. Your Webpack configuration is not creating a manifest. + MSG + raise(WebpackerLite::FileLoader::NotFoundError.new(msg)) + end + + def load_data return super unless File.exist?(@path) JSON.parse(File.read(@path)) end diff --git a/test/manifest_test.rb b/test/manifest_test.rb index edb536c..ec95b50 100644 --- a/test/manifest_test.rb +++ b/test/manifest_test.rb @@ -11,14 +11,14 @@ def test_lookup_exception asset_file = "calendar.js" msg = <<-MSG WebpackerLite can't find #{asset_file} in your manifest #{manifest_path}. Possible causes: - 1. You are hot reloading - 2. Webpack is not running + 1. You are hot reloading. + 2. Webpack has not re-run to reflect updates. 3. You have misconfigured WebpackerLite's config/webpacker_lite.yml file. 4. Your Webpack configuration is not creating a manifest. MSG error = assert_raises WebpackerLite::FileLoader::NotFoundError do - WebpackerLite::Manifest.lookup(asset_file) + WebpackerLite::Manifest.lookup!(asset_file) end assert_equal error.message, msg @@ -26,6 +26,6 @@ def test_lookup_exception def test_lookup_success asset_file = "bootstrap.js" - assert_equal WebpackerLite::Manifest.lookup(asset_file), "bootstrap-300631c4f0e0f9c865bc.js" + assert_equal WebpackerLite::Manifest.lookup!(asset_file), "bootstrap-300631c4f0e0f9c865bc.js" end end