diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cf322075..3a7a43f18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ Changes since last non-beta release. *Please add entries here for your pull requests that are not yet released.* +#### Fixed +- Fix test helper determination of stale assets. [PR 1090](https://github.com/shakacode/react_on_rails/pull/1090) by [justin808](https://github.com/justin808). + #### Changed ### [11.0.7] - 2018-05-11 #### Fixed diff --git a/docs/additional-reading/rspec-configuration.md b/docs/additional-reading/rspec-configuration.md index ee7f21b46..9ab2bdeaf 100644 --- a/docs/additional-reading/rspec-configuration.md +++ b/docs/additional-reading/rspec-configuration.md @@ -37,7 +37,10 @@ The following `config/react_on_rails.rb` settings **must** match your setup: # Define the files we need to check for webpack compilation when running tests. # Generally, the manifest.json is good enough for this check if using webpacker - config.webpack_generated_files = %w( hello-world-bundle.js ) + config.webpack_generated_files = %w( manifest.json ) + + # However if you're not hashing the server-bundle.js, then you should include your server-bundle.js in the list. + config.webpack_generated_files = %w( server-bundle.js manifest.json ) # If you are using the ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config) # with rspec then this controls what yarn command is run diff --git a/docs/basics/configuration.md b/docs/basics/configuration.md index 2e730a3cc..dc8569ded 100644 --- a/docs/basics/configuration.md +++ b/docs/basics/configuration.md @@ -58,8 +58,10 @@ ReactOnRails.configure do |config| # Define the files we need to check for webpack compilation when running tests. # The default is `%w( manifest.json )` as will be sufficient for most webpacker builds. + # However, if you are generated a server bundle that is NOT hashed (present in manifest.json), + # then include the file in this list like this: # - config.webpack_generated_files = %w( manifest.json ) + config.webpack_generated_files = %w( server-bundle.js manifest.json ) # You can optionally add values to your rails_context. See example below for RenderingExtension # config.rendering_extension = RenderingExtension @@ -75,8 +77,10 @@ ReactOnRails.configure do |config| # JavaScript execution instances which should handle any component requested. # # While you may configure this to be the same as your client bundle file, this file is typically - # different. - # + # different. Note, be sure to include the exact file name with the ".js" if you are not hashing this file. + # If you are hashing this file (supposing you are using the same file for client rendering), then + # + # you should include a name that matches your bundle name in your webpack config. config.server_bundle_js_file = "server-bundle.js" # If set to true, this forces Rails to reload the server bundle if it is modified diff --git a/docs/basics/upgrading-react-on-rails.md b/docs/basics/upgrading-react-on-rails.md index 070ebad1e..aed1f5076 100644 --- a/docs/basics/upgrading-react-on-rails.md +++ b/docs/basics/upgrading-react-on-rails.md @@ -25,10 +25,10 @@ Reason for doing this: This enables your webpack bundles to bypass the Rails ass #### From version 7 or lower ##### ...while keeping your `client` directory -Unfortunately, this requires quite a few steps: * `.gitignore`: add `/public/webpack/*` * `Gemfile`: bump `react_on_rails` and add `webpacker` -* layout views: anything bundled by webpack will need to be requested by a `javascript_pack_tag` or `stylesheet_pack_tag` +* layout views: anything bundled by webpack will need to be requested by a `javascript_pack_tag` or `stylesheet_pack_tag`. + * Search your codebase for javascript_include_tag. Use the * `config/initializers/assets.rb`: we no longer need to modify `Rails.application.config.assets.paths` or append anything to `Rails.application.config.assets.precompile`. * `config/initializers/react_on_rails.rb`: * Delete `config.generated_assets_dir`. Webpacker's config now supplies this information diff --git a/lib/react_on_rails/configuration.rb b/lib/react_on_rails/configuration.rb index 942982b7f..138e5b17a 100644 --- a/lib/react_on_rails/configuration.rb +++ b/lib/react_on_rails/configuration.rb @@ -10,6 +10,8 @@ def self.configure DEFAULT_SERVER_RENDER_TIMEOUT = 20 DEFAULT_POOL_SIZE = 1 + # TODO: Move this inside of the class Configuration so as not to put all these methods + # on ReactOnRails def self.setup_config_values ensure_webpack_generated_files_exists configure_generated_assets_dirs_deprecation diff --git a/lib/react_on_rails/prerender_error.rb b/lib/react_on_rails/prerender_error.rb index 64a53be81..f60f20adb 100644 --- a/lib/react_on_rails/prerender_error.rb +++ b/lib/react_on_rails/prerender_error.rb @@ -3,6 +3,8 @@ # rubocop:disable: Layout/IndentHeredoc module ReactOnRails class PrerenderError < ::ReactOnRails::Error + # TODO: Consider remove providing original `err` as already have access to `self.cause` + # http://blog.honeybadger.io/nested-errors-in-ruby-with-exception-cause/ attr_reader :component_name, :err, :props, :js_code, :console_messages # err might be nil if JS caught the error diff --git a/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb b/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb index bcb0cc934..a58077300 100644 --- a/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb +++ b/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb @@ -53,7 +53,11 @@ def find_most_recent_mtime def all_compiled_assets @all_compiled_assets ||= begin webpack_generated_files = @webpack_generated_files.map do |bundle_name| - ReactOnRails::Utils.bundle_js_file_path(bundle_name) + if bundle_name == ReactOnRails.configuration.server_bundle_js_file + ReactOnRails::Utils.server_bundle_js_file_path + else + ReactOnRails::Utils.bundle_js_file_path(bundle_name) + end end if webpack_generated_files.present? diff --git a/spec/dummy/client/webpack.client.rails.build.config.js b/spec/dummy/client/webpack.client.rails.build.config.js index 232751ee8..170cd73bd 100644 --- a/spec/dummy/client/webpack.client.rails.build.config.js +++ b/spec/dummy/client/webpack.client.rails.build.config.js @@ -4,7 +4,7 @@ const ExtractTextPlugin = require('extract-text-webpack-plugin'); const merge = require('webpack-merge'); -const { env } = require('process') +const { env } = require('process'); const config = require('./webpack.client.base.config'); const { resolve } = require('path'); @@ -14,7 +14,7 @@ const configPath = resolve('..', 'config'); const { output, settings } = webpackConfigLoader(configPath); const isHMR = settings.dev_server && settings.dev_server.hmr -const devBuild = process.env.NODE_ENV !== 'production'; +const devBuild = env.NODE_ENV !== 'production'; if (devBuild) { diff --git a/spec/react_on_rails/configuration_spec.rb b/spec/react_on_rails/configuration_spec.rb index 4d3ec8f00..f5cf27b87 100644 --- a/spec/react_on_rails/configuration_spec.rb +++ b/spec/react_on_rails/configuration_spec.rb @@ -8,6 +8,10 @@ module ReactOnRails let(:not_existing_path) { "/path/to/#{SecureRandom.hex(4)}" } let(:using_webpacker) { false } + after do + ReactOnRails.instance_variable_set(:@configuration, nil) + end + before do allow(ReactOnRails::WebpackerUtils).to receive(:using_webpacker?).and_return(using_webpacker) end diff --git a/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/client/myApp.jsx b/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/client/myApp.jsx new file mode 100644 index 000000000..e69de29bb diff --git a/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/compiled_js/client-bundle-6bc530d039d96709b68d.js b/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/compiled_js/client-bundle-6bc530d039d96709b68d.js new file mode 100644 index 000000000..e69de29bb diff --git a/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/compiled_js/manifest.json b/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/compiled_js/manifest.json new file mode 100644 index 000000000..5de50e317 --- /dev/null +++ b/spec/react_on_rails/fixtures/webpack_assets/assets_with_manifest_exist_server_bundle_separate/compiled_js/manifest.json @@ -0,0 +1,3 @@ +{ + "client-bundle.js": "client-bundle-6bc530d039d96709b68d.js" +} diff --git a/spec/react_on_rails/test_helper/webpack_assets_status_checker_spec.rb b/spec/react_on_rails/test_helper/webpack_assets_status_checker_spec.rb index c17a66de8..1973b9753 100644 --- a/spec/react_on_rails/test_helper/webpack_assets_status_checker_spec.rb +++ b/spec/react_on_rails/test_helper/webpack_assets_status_checker_spec.rb @@ -45,7 +45,7 @@ .and_return(File.join(generated_assets_full_path, "client-bundle-6bc530d039d96709b68d.js")) allow(ReactOnRails::Utils).to receive(:bundle_js_file_path) .with("server-bundle.js") - .and_return(File.join(generated_assets_full_path, "server-bundle-6bc530d039d96702268d.js")) + .and_return(File.join(generated_assets_full_path, "server-bundle.js")) touch_files_in_dir(generated_assets_full_path) end @@ -63,6 +63,31 @@ specify { expect(checker.stale_generated_webpack_files).to eq(["manifest.json"]) } end + context "when using webpacker, a missing server bundle without hash, and manifest is current" do + let(:fixture_dirname) { "assets_with_manifest_exist_server_bundle_separate" } + + before do + require "webpacker" + allow(ReactOnRails::WebpackerUtils).to receive(:using_webpacker?).and_return(true) + allow(ReactOnRails::WebpackerUtils).to receive(:manifest_exists?).and_return(true) + allow(ReactOnRails::WebpackerUtils).to receive(:webpacker_public_output_path) + .and_return(generated_assets_full_path) + allow(ReactOnRails.configuration).to receive(:server_bundle_js_file).and_return("server-bundle.js") + allow(ReactOnRails::Utils).to receive(:bundle_js_file_path) + .with("client-bundle.js") + .and_return(File.join(generated_assets_full_path, "client-bundle-6bc530d039d96709b68d.js")) + allow(ReactOnRails::Utils).to receive(:bundle_js_file_path) + .with("server-bundle.js") + .and_raise(Webpacker::Manifest::MissingEntryError) + touch_files_in_dir(generated_assets_full_path) + end + + specify do + expect(checker.stale_generated_webpack_files.first) + .to match(/server-bundle\.js$/) + end + end + context "No Webpacker" do before do allow(ReactOnRails::WebpackerUtils).to receive(:using_webpacker?).and_return(false)