Refactor and minor improvements (#1280)

* Update gemfiles with x86_64-linux deps

* Improve style

* Simplify method body

* Improve file name and required packages

* Add commented entry in common config for ignoring warning

Author: ahangarha

gemfiles/no_sprockets.gemfile.lock

@@ -169,6 +169,8 @@ GEM
     nio4r (2.5.9)
     nokogiri (1.13.8-x86_64-darwin)
       racc (~> 1.4)
+    nokogiri (1.13.8-x86_64-linux)
+      racc (~> 1.4)
     notiffany (0.1.3)
       nenv (~> 0.1)
       shellany (~> 0.0)
@@ -243,6 +245,7 @@ GEM
 
 PLATFORMS
   x86_64-darwin-20
+  x86_64-linux
 
 DEPENDENCIES
   appraisal

gemfiles/no_sprockets_shakapacker.gemfile.lock

@@ -171,6 +171,8 @@ GEM
     nio4r (2.5.9)
     nokogiri (1.14.3-x86_64-darwin)
       racc (~> 1.4)
+    nokogiri (1.14.3-x86_64-linux)
+      racc (~> 1.4)
     notiffany (0.1.3)
       nenv (~> 0.1)
       shellany (~> 0.0)
@@ -256,6 +258,7 @@ GEM
 
 PLATFORMS
   x86_64-darwin-20
+  x86_64-linux
 
 DEPENDENCIES
   appraisal

gemfiles/sprockets_3.gemfile.lock

@@ -141,6 +141,7 @@ GEM
       activesupport (>= 4.2.0)
     json (2.3.0)
     libv8 (7.3.492.27.1-x86_64-darwin-20)
+    libv8 (7.3.492.27.1-x86_64-linux)
     listen (3.0.8)
       rb-fsevent (~> 0.9, >= 0.9.4)
       rb-inotify (~> 0.9, >= 0.9.7)
@@ -172,6 +173,8 @@ GEM
     nio4r (2.5.9)
     nokogiri (1.13.8-x86_64-darwin)
       racc (~> 1.4)
+    nokogiri (1.13.8-x86_64-linux)
+      racc (~> 1.4)
     notiffany (0.1.3)
       nenv (~> 0.1)
       shellany (~> 0.0)
@@ -256,6 +259,7 @@ GEM
 
 PLATFORMS
   x86_64-darwin-20
+  x86_64-linux
 
 DEPENDENCIES
   appraisal

gemfiles/sprockets_4.gemfile.lock

@@ -141,6 +141,7 @@ GEM
       activesupport (>= 4.2.0)
     json (2.3.0)
     libv8 (7.3.492.27.1-x86_64-darwin-20)
+    libv8 (7.3.492.27.1-x86_64-linux)
     listen (3.0.8)
       rb-fsevent (~> 0.9, >= 0.9.4)
       rb-inotify (~> 0.9, >= 0.9.7)
@@ -172,6 +173,8 @@ GEM
     nio4r (2.5.9)
     nokogiri (1.13.8-x86_64-darwin)
       racc (~> 1.4)
+    nokogiri (1.13.8-x86_64-linux)
+      racc (~> 1.4)
     notiffany (0.1.3)
       nenv (~> 0.1)
       shellany (~> 0.0)
@@ -256,6 +259,7 @@ GEM
 
 PLATFORMS
   x86_64-darwin-20
+  x86_64-linux
 
 DEPENDENCIES
   appraisal

lib/react/server_rendering/bundle_renderer.rb

@@ -71,11 +71,11 @@ def asset_container
       def prepare_options(options)
         r_func = render_function(options)
         opts = case options
-          when Hash then options
-          when TrueClass then {}
-          else
-            {}
-        end
+               when Hash then options
+               when TrueClass then {}
+               else
+                 {}
+               end
         # This seems redundant to pass
         opts.merge(render_function: r_func)
       end
@@ -100,22 +100,16 @@ def assets_precompiled?
       # Or, if the user has provided {.asset_container_class}, use that.
       # @return [Class] suitable for {#asset_container}
       def asset_container_class
-        if self.class.asset_container_class.present?
-          self.class.asset_container_class
-        elsif SeparateServerBundleContainer.compatible?
-          SeparateServerBundleContainer
-        elsif assets_precompiled?
-          if ManifestContainer.compatible?
-            ManifestContainer
-          elsif YamlManifestContainer.compatible?
-            YamlManifestContainer
-          else
-            # Even though they are precompiled, we can't find them :S
-            EnvironmentContainer
-          end
-        else
-          EnvironmentContainer
-        end
+        return self.class.asset_container_class if self.class.asset_container_class.present?
+        return SeparateServerBundleContainer if SeparateServerBundleContainer.compatible?
+
+        return EnvironmentContainer unless assets_precompiled?
+
+        return ManifestContainer if ManifestContainer.compatible?
+        return YamlManifestContainer if YamlManifestContainer.compatible?
+
+        # Even though they are precompiled, we can't find them :S
+        EnvironmentContainer
       end
     end
   end

test/dummy/config/webpack/commonWebpackConfig.js

@@ -17,7 +17,9 @@ const commonOptions = {
         ]
       }
     ]
-  }
+  },
+  // Uncommemt if getting "Module not found: Error: Can't resolve 'react-dom/client'" warning
+  // ignoreWarnings: [/Module not found: Error: Can't resolve 'react-dom\/client'/]
 }
 
 // Copy the object using merge b/c the baseClientWebpackConfig and commonOptions are mutable globals

test/dummy/config/webpack/development.js

@@ -1,7 +1,6 @@
-const path = require('path')
 const { devServer, inliningCss } = require('shakapacker')
 
-const webpackConfig = require('./ServerClientOrBoth')
+const webpackConfig = require('./serverClientOrBoth')
 
 const developmentEnvOnly = (clientWebpackConfig, serverWebpackConfig) => {
 

test/dummy/config/webpack/production.js

@@ -1,6 +1,6 @@
 // Below code should get refactored but the current way that rails/webpacker v5
 // does the globals, it's tricky
-const webpackConfig = require('./ServerClientOrBoth')
+const webpackConfig = require('./serverClientOrBoth')
 
 const productionEnvOnly = (_clientWebpackConfig, _serverWebpackConfig) => {
   // place any code here that is for production only

test/dummy/config/webpack/ServerClientOrBoth.js renamed to test/dummy/config/webpack/serverClientOrBoth.js

@@ -1,8 +1,8 @@
+const webpack = require('webpack')
+
 const { merge, config } = require('shakapacker')
 const commonWebpackConfig = require('./commonWebpackConfig')
 
-const webpack = require('webpack')
-
 const configureServer = () => {
   // We need to use "merge" because the clientConfigObject, EVEN after running
   // toWebpackConfig() is a mutable GLOBAL. Thus any changes, like modifying the

test/dummy/config/webpack/serverWebpackConfig.js

@@ -1,4 +1,4 @@
-const webpackConfig = require('./ServerClientOrBoth')
+const webpackConfig = require('./serverClientOrBoth')
 
 const testOnly = (_clientWebpackConfig, _serverWebpackConfig) => {
   // place any code here that is for test only

test/dummy/config/webpack/test.js

@@ -1,6 +1,6 @@
-const { env, webpackConfig } = require('shakapacker');
 const { existsSync } = require('fs');
 const { resolve } = require('path');
+const { env, webpackConfig } = require('shakapacker');
 
 const envSpecificConfig = () => {
   const path = resolve(__dirname, `${env.nodeEnv}.js`);