Skip to content

Refactor and minor improvements #1280

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

Merged
merged 5 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions gemfiles/no_sprockets.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -243,6 +245,7 @@ GEM

PLATFORMS
x86_64-darwin-20
x86_64-linux

DEPENDENCIES
appraisal
Expand Down
3 changes: 3 additions & 0 deletions gemfiles/no_sprockets_shakapacker.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -256,6 +258,7 @@ GEM

PLATFORMS
x86_64-darwin-20
x86_64-linux

DEPENDENCIES
appraisal
Expand Down
4 changes: 4 additions & 0 deletions gemfiles/sprockets_3.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -256,6 +259,7 @@ GEM

PLATFORMS
x86_64-darwin-20
x86_64-linux

DEPENDENCIES
appraisal
Expand Down
4 changes: 4 additions & 0 deletions gemfiles/sprockets_4.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -256,6 +259,7 @@ GEM

PLATFORMS
x86_64-darwin-20
x86_64-linux

DEPENDENCIES
appraisal
Expand Down
36 changes: 15 additions & 21 deletions lib/react/server_rendering/bundle_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion test/dummy/config/webpack/commonWebpackConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions test/dummy/config/webpack/development.js
Original file line number Diff line number Diff line change
@@ -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) => {

Expand Down
2 changes: 1 addition & 1 deletion test/dummy/config/webpack/production.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/dummy/config/webpack/serverWebpackConfig.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/config/webpack/test.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/config/webpack/webpack.config.js
Original file line number Diff line number Diff line change
@@ -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`);
Expand Down