Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Shakapacker v7 deprecation messages #1592

Merged
merged 13 commits into from
Apr 3, 2024
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ Changes since the last non-beta release.
- Dropped Ruby 2.7 support [PR 1595](https://github.com/shakacode/react_on_rails/pull/1595) by [ahangarha](https://github.com/ahangarha).
- Removed deprecated `webpackConfigLoader.js` [PR 1600](https://github.com/shakacode/react_on_rails/pull/1600) by [ahangarha](https://github.com/ahangarha).

#### Fixed
- Prevent displaying the deprecation message for using `webpacker_precompile?` method and `webpacker:clean` rake task when using Shakapacker v7+ [PR 1592](https://github.com/shakacode/react_on_rails/pull/1592) by [ahangarha](https://github.com/ahangarha).

### [13.4.1]
#### Fixed
- Fixed Typescript types for ServerRenderResult, ReactComponent, RenderFunction, and RailsContext interfaces. [PR 1582](https://github.com/shakacode/react_on_rails/pull/1582) & [PR 1585](https://github.com/shakacode/react_on_rails/pull/1585) by [kotarella1110](https://github.com/kotarella1110)
Expand Down
46 changes: 30 additions & 16 deletions lib/react_on_rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,31 +141,19 @@ def adjust_precompile_task

return if skip_react_on_rails_precompile || build_production_command.blank?

if Webpacker.config.webpacker_precompile?
msg = <<~MSG

React on Rails and Shakapacker error in configuration!
In order to use config/react_on_rails.rb config.build_production_command,
you must edit config/webpacker.yml to include this value in the default configuration:
'webpacker_precompile: false'

Alternatively, remove the config/react_on_rails.rb config.build_production_command and the
default bin/webpacker script will be used for assets:precompile.

MSG
raise ReactOnRails::Error, msg
end
raise(ReactOnRails::Error, compile_command_conflict_message) if shakapacker_precompile?

precompile_tasks = lambda {
Rake::Task["react_on_rails:generate_packs"].invoke
Rake::Task["react_on_rails:assets:webpack"].invoke
puts "Invoking task webpacker:clean from React on Rails"

# VERSIONS is per the shakacode/shakapacker clean method definition.
# We set it very big so that it is not used, and then clean just
# removes files older than 1 hour.
versions = 100_000
Rake::Task["webpacker:clean"].invoke(versions)

puts "Invoking task #{shakapacker_clean_task} from React on Rails"
Rake::Task[shakapacker_clean_task].invoke(versions)
}

if Rake::Task.task_defined?("assets:precompile")
Expand Down Expand Up @@ -266,6 +254,32 @@ def raise_missing_components_subdirectory

raise ReactOnRails::Error, msg
end

def shakapacker_precompile?
return Webpacker.config.webpacker_precompile? if ReactOnRails::WebpackerUtils.using_shakapacker_6?

Webpacker.config.shakapacker_precompile?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahangarha @Judahmeek should this be

Shakapacker.config.shakapacker_precompile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should do this when we drop support for Webpacker/Shakapacker 6.

This way, we still know we are supporting the old interface. Otherwise, changing the name is not a big issue and can be done in a separate PR.

end

def shakapacker_clean_task
ReactOnRails::WebpackerUtils.using_shakapacker_6? ? "webpacker:clean" : "shakapacker:clean"
end

def compile_command_conflict_message
packer = ReactOnRails::WebpackerUtils.using_shakapacker_6? ? "webpacker" : "shakapacker"

<<~MSG

React on Rails and Shakapacker error in configuration!
In order to use config/react_on_rails.rb config.build_production_command,
you must edit config/#{packer}.yml to include this value in the default configuration:
'#{packer}_precompile: false'

Alternatively, remove the config/react_on_rails.rb config.build_production_command and the
default bin/#{packer} script will be used for assets:precompile.

MSG
end
end
end
# rubocop:enable Metrics/ClassLength
6 changes: 6 additions & 0 deletions lib/react_on_rails/webpacker_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,5 +141,11 @@ def self.raise_shakapacker_not_installed
def self.semver_to_string(ary)
"#{ary[0]}.#{ary[1]}.#{ary[2]}"
end

def self.using_shakapacker_6?
shakapacker_major_version = shakapacker_version_as_array[0]

shakapacker_major_version == 6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shakapacker_version[:major] == 6

isn't that nicer? you can change the shakapacker_version method very easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if it is okay to modify that method, your suggestion is much much better.
Can we be sure that nobody uses that method?

end
end
end
25 changes: 25 additions & 0 deletions spec/dummy/spec/rake/assets_precompile_rake_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

require "rake"

require "rails_helper"

describe "rake assets:precompile task" do
it "doesn't show deprecation message for using webpacker:clean task" do
allow(ENV).to receive(:[]).with(anything).and_call_original
allow(ENV).to receive(:[]).with("SHAKAPACKER_PRECOMPILE").and_return("false")

Rails.application.load_tasks

ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/shakapacker"
end

expect do
system "bundle install && yarn install && yarn run build:test"
Rake::Task["assets:precompile"].execute
end.not_to output(/Consider using `rake shakapacker:clean`/).to_stdout

Rake::Task.clear
end
end
123 changes: 93 additions & 30 deletions spec/react_on_rails/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,40 +75,103 @@ module ReactOnRails
end

describe ".build_production_command" do
it "fails when \"webpacker_precompile\" is truly and \"build_production_command\" is truly" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(true)
expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/webpacker"
end
end.to raise_error(ReactOnRails::Error, /webpacker_precompile: false/)
end
context "when using Shakapacker 6" do
before do
allow(ReactOnRails::WebpackerUtils)
.to receive("shakapacker_version")
.and_return("6.0.0")
end

it "doesn't fail when \"webpacker_precompile\" is falsy and \"build_production_command\" is truly" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(false)
expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/webpacker"
end
end.not_to raise_error
end
it "fails when \"webpacker_precompile\" is truly and \"build_production_command\" is truly" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(true)
expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/webpacker"
end
end.to raise_error(ReactOnRails::Error, /webpacker_precompile: false/)
end

it "doesn't fail when \"webpacker_precompile\" is truly and \"build_production_command\" is falsy" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(true)
expect do
ReactOnRails.configure {} # rubocop:disable-line Lint/EmptyBlock
end.not_to raise_error
it "doesn't fail when \"webpacker_precompile\" is falsy and \"build_production_command\" is truly" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(false)
expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/webpacker"
end
end.not_to raise_error
end

it "doesn't fail when \"webpacker_precompile\" is truly and \"build_production_command\" is falsy" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(true)
expect do
ReactOnRails.configure {} # rubocop:disable-line Lint/EmptyBlock
end.not_to raise_error
end

it "doesn't fail when \"webpacker_precompile\" is falsy and \"build_production_command\" is falsy" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(false)
expect do
ReactOnRails.configure {} # rubocop:disable-line Lint/EmptyBlock
end.not_to raise_error
end
end

it "doesn't fail when \"webpacker_precompile\" is falsy and \"build_production_command\" is falsy" do
allow(Webpacker).to receive_message_chain("config.webpacker_precompile?")
.and_return(false)
expect do
ReactOnRails.configure {} # rubocop:disable-line Lint/EmptyBlock
end.not_to raise_error
context "when using Shakapacker 7" do
before do
allow(ReactOnRails::WebpackerUtils)
.to receive("shakapacker_version")
.and_return("7.0.0")
end

it "doesn't show deprecation message for webpacker_precompile?" do
allow(Webpacker).to receive_message_chain("config.shakapacker_precompile?")
.and_return(false)

expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/shakapacker"
end
end.not_to output(/Consider using `shakapacker_precompile?`/).to_stdout
end

it "fails when \"shakapacker_precompile\" is truly and \"build_production_command\" is truly" do
allow(Webpacker).to receive_message_chain("config.shakapacker_precompile?")
.and_return(true)
expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/shakapacker"
end
end.to raise_error(ReactOnRails::Error, /shakapacker_precompile: false/)
end

it "doesn't fail when \"shakapacker_precompile\" is falsy and \"build_production_command\" is truly" do
allow(Webpacker).to receive_message_chain("config.shakapacker_precompile?")
.and_return(false)
expect do
ReactOnRails.configure do |config|
config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/shakapacker"
end
end.not_to raise_error
end

it "doesn't fail when \"shakapacker_precompile\" is truly and \"build_production_command\" is falsy" do
allow(Webpacker).to receive_message_chain("config.shakapacker_precompile?")
.and_return(true)
expect do
ReactOnRails.configure {} # rubocop:disable-line Lint/EmptyBlock
end.not_to raise_error
end

it "doesn't fail when \"shakapacker_precompile\" is falsy and \"build_production_command\" is falsy" do
allow(Webpacker).to receive_message_chain("config.shakapacker_precompile?")
.and_return(false)
expect do
ReactOnRails.configure {} # rubocop:disable-line Lint/EmptyBlock
end.not_to raise_error
end
end
end

Expand Down
12 changes: 6 additions & 6 deletions spec/react_on_rails/generators/dev_tests_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@

it "changes package.json to use local react-on-rails version of module" do
assert_file("package.json") do |contents|
assert_match('"react-on-rails"', contents)
assert_match('"postinstall"', contents)
expect(contents).to match('"react-on-rails"')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, such changes are in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. But it was needed to pass Rubocop.

expect(contents).to match('"postinstall"')
end
end

it "adds test-related gems to Gemfile" do
assert_file("Gemfile") do |contents|
assert_match("gem \"rspec-rails\", group: :test", contents)
assert_match("gem \"coveralls\", require: false", contents)
assert_match("gem \"chromedriver-helper\", group: :test", contents)
expect(contents).to match("gem \"rspec-rails\", group: :test")
expect(contents).to match("gem \"coveralls\", require: false")
expect(contents).to match("gem \"chromedriver-helper\", group: :test")
end
end
end
Expand All @@ -49,7 +49,7 @@

it "adds prerender for examples with example-server-rendering" do
assert_file("app/views/hello_world/index.html.erb") do |contents|
assert_match("prerender: true", contents)
expect(contents).to match("prerender: true")
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/react_on_rails/generators/install_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

it "adds ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)" do
expected = ReactOnRails::Generators::BaseGenerator::CONFIGURE_RSPEC_TO_COMPILE_ASSETS
assert_file("spec/rails_helper.rb") { |contents| assert_match(expected, contents) }
assert_file("spec/rails_helper.rb") { |contents| expect(contents).to match(expected) }
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
shared_examples "no_redux_generator" do
it "creates appropriate templates" do
assert_file("app/javascript/packs/hello-world-bundle.js") do |contents|
assert_match("import HelloWorld from '../bundles/HelloWorld/components/HelloWorld';", contents)
expect(contents).to match("import HelloWorld from '../bundles/HelloWorld/components/HelloWorld';")
end

assert_file("app/views/hello_world/index.html.erb") do |contents|
assert_match(/"HelloWorld"/, contents)
expect(contents).to match(/"HelloWorld"/)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

it "creates appropriate templates" do
assert_file("app/javascript/packs/hello-world-bundle.js") do |contents|
assert_match("import HelloWorldApp from '../bundles/HelloWorld/startup/HelloWorldApp';", contents)
expect(contents).to match("import HelloWorldApp from '../bundles/HelloWorld/startup/HelloWorldApp';")
end
assert_file("app/views/hello_world/index.html.erb") do |contents|
assert_match(/"HelloWorldApp"/, contents)
expect(contents).to match(/"HelloWorldApp"/)
end
end

Expand Down
Loading