-
Notifications
You must be signed in to change notification settings - Fork 380
Update to Shakapacker 9.1.0 and migrate to Rspack #680
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
Changes from 5 commits
a951f4c
879d171
087ec70
5d85f15
3fe61f0
fbc5781
76921b8
012b0b7
1685fb4
28014b2
3da3dfc
71b934a
752919b
4c761bb
431a8ee
2e03f56
5f92988
0ab9eac
a32ebff
84311cc
660aab3
2bdc624
2af9d6f
4d9d19e
13449f0
b7171e5
8bae4aa
ab8bd51
921844d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
// Default locale and messages for i18n | ||
export const defaultLocale = 'en'; | ||
|
||
export const defaultMessages = { | ||
'app.name': 'React Webpack Rails Tutorial', | ||
'comment.form.name_label': 'Name', | ||
'comment.form.text_label': 'Text', | ||
'comment.form.submit': 'Submit', | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// Translation messages for different locales | ||
export const translations = { | ||
en: { | ||
'app.name': 'React Webpack Rails Tutorial', | ||
'comment.form.name_label': 'Name', | ||
'comment.form.text_label': 'Text', | ||
'comment.form.submit': 'Submit', | ||
}, | ||
es: { | ||
'app.name': 'Tutorial de React Webpack Rails', | ||
'comment.form.name_label': 'Nombre', | ||
'comment.form.text_label': 'Texto', | ||
'comment.form.submit': 'Enviar', | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
const { resolve } = require('path'); | ||
|
||
module.exports = { | ||
resolve: { | ||
alias: { | ||
Assets: resolve(__dirname, '..', '..', 'client', 'app', 'assets'), | ||
}, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
const rspack = require('@rspack/core'); | ||
const commonRspackConfig = require('./commonRspackConfig'); | ||
|
||
const configureClient = () => { | ||
const clientConfig = commonRspackConfig(); | ||
|
||
clientConfig.plugins.push( | ||
new rspack.ProvidePlugin({ | ||
$: 'jquery', | ||
jQuery: 'jquery', | ||
ActionCable: '@rails/actioncable', | ||
}), | ||
); | ||
|
||
// server-bundle is special and should ONLY be built by the serverConfig | ||
// In case this entry is not deleted, a very strange "window" not found | ||
// error shows referring to window["webpackJsonp"]. That is because the | ||
// client config is going to try to load chunks. | ||
delete clientConfig.entry['server-bundle']; | ||
|
||
return clientConfig; | ||
}; | ||
|
||
module.exports = configureClient; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// Common configuration applying to client and server configuration | ||
const { generateWebpackConfig, merge } = require('shakapacker'); | ||
|
||
const baseClientRspackConfig = generateWebpackConfig(); | ||
const commonOptions = { | ||
resolve: { | ||
extensions: ['.css', '.ts', '.tsx'], | ||
}, | ||
}; | ||
|
||
// add sass resource loader | ||
const sassLoaderConfig = { | ||
loader: 'sass-resources-loader', | ||
options: { | ||
resources: './client/app/assets/styles/app-variables.scss', | ||
}, | ||
}; | ||
|
||
const ignoreWarningsConfig = { | ||
ignoreWarnings: [/Module not found: Error: Can't resolve 'react-dom\/client'/], | ||
}; | ||
|
||
const scssConfigIndex = baseClientRspackConfig.module.rules.findIndex((config) => | ||
'.scss'.match(config.test) && config.use, | ||
); | ||
|
||
if (scssConfigIndex === -1) { | ||
console.warn('No SCSS rule with use array found in rspack config'); | ||
} else { | ||
// Configure sass-loader to use the modern API | ||
const scssRule = baseClientRspackConfig.module.rules[scssConfigIndex]; | ||
const sassLoaderIndex = scssRule.use.findIndex((loader) => { | ||
if (typeof loader === 'string') { | ||
return loader.includes('sass-loader'); | ||
} | ||
return loader.loader && loader.loader.includes('sass-loader'); | ||
}); | ||
|
||
if (sassLoaderIndex !== -1) { | ||
const sassLoader = scssRule.use[sassLoaderIndex]; | ||
if (typeof sassLoader === 'string') { | ||
scssRule.use[sassLoaderIndex] = { | ||
loader: sassLoader, | ||
options: { | ||
api: 'modern' | ||
} | ||
}; | ||
} else { | ||
sassLoader.options = sassLoader.options || {}; | ||
sassLoader.options.api = 'modern'; | ||
} | ||
} | ||
|
||
// Fix css-loader configuration for CSS modules if namedExport is enabled | ||
// When namedExport is true, exportLocalsConvention must be camelCaseOnly or dashesOnly | ||
const cssLoader = scssRule.use.find((loader) => { | ||
const loaderName = typeof loader === 'string' ? loader : loader?.loader; | ||
return loaderName?.includes('css-loader'); | ||
}); | ||
|
||
if (cssLoader?.options?.modules?.namedExport) { | ||
cssLoader.options.modules.exportLocalsConvention = 'camelCaseOnly'; | ||
} | ||
|
||
baseClientRspackConfig.module.rules[scssConfigIndex].use.push(sassLoaderConfig); | ||
} | ||
|
||
|
||
// Copy the object using merge b/c the baseClientRspackConfig and commonOptions are mutable globals | ||
const commonRspackConfig = () => merge({}, baseClientRspackConfig, commonOptions, ignoreWarningsConfig); | ||
|
||
module.exports = commonRspackConfig; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
process.env.NODE_ENV = process.env.NODE_ENV || 'development'; | ||
|
||
const { devServer, inliningCss } = require('shakapacker'); | ||
|
||
const rspackConfig = require('./rspackConfig'); | ||
|
||
const developmentEnvOnly = (clientRspackConfig, _serverRspackConfig) => { | ||
// plugins | ||
if (inliningCss) { | ||
// Note, when this is run, we're building the server and client bundles in separate processes. | ||
// Thus, this plugin is not applied to the server bundle. | ||
|
||
// eslint-disable-next-line global-require | ||
const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin'); | ||
clientRspackConfig.plugins.push( | ||
new ReactRefreshWebpackPlugin({ | ||
overlay: { | ||
sockPort: devServer.port, | ||
}, | ||
}), | ||
); | ||
} | ||
}; | ||
|
||
module.exports = rspackConfig(developmentEnvOnly); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
process.env.NODE_ENV = process.env.NODE_ENV || 'production'; | ||
|
||
const rspackConfig = require('./rspackConfig'); | ||
|
||
const productionEnvOnly = (_clientRspackConfig, _serverRspackConfig) => { | ||
// place any code here that is for production only | ||
}; | ||
|
||
module.exports = rspackConfig(productionEnvOnly); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
const { env, generateWebpackConfig } = require('shakapacker'); | ||
const { existsSync } = require('fs'); | ||
const { resolve } = require('path'); | ||
|
||
const envSpecificConfig = () => { | ||
const path = resolve(__dirname, `${env.nodeEnv}.js`); | ||
if (existsSync(path)) { | ||
console.log(`Loading ENV specific rspack configuration file ${path}`); | ||
return require(path); | ||
} | ||
|
||
return generateWebpackConfig(); | ||
}; | ||
|
||
module.exports = envSpecificConfig(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
const clientRspackConfig = require('./clientRspackConfig'); | ||
const serverRspackConfig = require('./serverRspackConfig'); | ||
|
||
const rspackConfig = (envSpecific) => { | ||
const clientConfig = clientRspackConfig(); | ||
const serverConfig = serverRspackConfig(); | ||
|
||
if (envSpecific) { | ||
envSpecific(clientConfig, serverConfig); | ||
} | ||
|
||
let result; | ||
// For HMR, need to separate the the client and server rspack configurations | ||
if (process.env.WEBPACK_SERVE || process.env.CLIENT_BUNDLE_ONLY) { | ||
// eslint-disable-next-line no-console | ||
console.log('[React on Rails] Creating only the client bundles.'); | ||
result = clientConfig; | ||
} else if (process.env.SERVER_BUNDLE_ONLY) { | ||
// eslint-disable-next-line no-console | ||
console.log('[React on Rails] Creating only the server bundle.'); | ||
result = serverConfig; | ||
} else { | ||
// default is the standard client and server build | ||
// eslint-disable-next-line no-console | ||
console.log('[React on Rails] Creating both client and server bundles.'); | ||
result = [clientConfig, serverConfig]; | ||
} | ||
|
||
// To debug, uncomment next line and inspect "result" | ||
// debugger | ||
return result; | ||
}; | ||
|
||
module.exports = rspackConfig; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,122 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
const path = require('path'); | ||||||||||||||||||||||||||||||||||||||||||||||||||
const { config } = require('shakapacker'); | ||||||||||||||||||||||||||||||||||||||||||||||||||
const commonRspackConfig = require('./commonRspackConfig'); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
const rspack = require('@rspack/core'); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
const path = require('path'); | |
const { config } = require('shakapacker'); | |
const commonRspackConfig = require('./commonRspackConfig'); | |
const rspack = require('@rspack/core'); | |
const path = require('path'); | |
const { config } = require('shakapacker'); | |
const rspack = require('@rspack/core'); | |
const commonRspackConfig = require('./commonRspackConfig'); | |
const configureServer = () => { | |
// …rest of implementation… | |
}; |
🧰 Tools
🪛 ESLint
[error] 5-5: @rspack/core
import should occur before import of ./commonRspackConfig
(import/order)
🤖 Prompt for AI Agents
In config/rspack/serverRspackConfig.js lines 1-6, the import order violates
ESLint import/order by placing a local module before an external package;
reorder requires so that external packages (e.g., path, @rspack/core,
shakapacker) come first and local modules (./commonRspackConfig) come after,
keeping existing declarations the same but moving the const rspack =
require('@rspack/core') line up with the other external requires and leaving
./commonRspackConfig below them.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate mini-css-extract-plugin filtering.
This filtering logic is duplicated at lines 67-107, where it's handled more comprehensively along with other SSR-specific loader adjustments. The duplicate code reduces maintainability without adding value.
Apply this diff:
serverRspackConfig.entry = serverEntry;
- // Remove the mini-css-extract-plugin from the style loaders because
- // the client build will handle exporting CSS.
- // replace file-loader with null-loader
- serverRspackConfig.module.rules.forEach((loader) => {
- if (loader.use && loader.use.filter) {
- loader.use = loader.use.filter(
- (item) => !(typeof item === 'string' && item.match(/mini-css-extract-plugin/)),
- );
- }
- });
-
// No splitting of chunks for a server bundle
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Remove the mini-css-extract-plugin from the style loaders because | |
// the client build will handle exporting CSS. | |
// replace file-loader with null-loader | |
serverRspackConfig.module.rules.forEach((loader) => { | |
if (loader.use && loader.use.filter) { | |
loader.use = loader.use.filter( | |
(item) => !(typeof item === 'string' && item.match(/mini-css-extract-plugin/)), | |
); | |
} | |
}); | |
serverRspackConfig.entry = serverEntry; | |
// No splitting of chunks for a server bundle |
🧰 Tools
🪛 ESLint
[error] 32-32: Assignment to property of function parameter 'loader'.
(no-param-reassign)
🤖 Prompt for AI Agents
In config/rspack/serverRspackConfig.js around lines 27 to 36, remove the
duplicate block that filters out mini-css-extract-plugin from loader.use — this
logic is already implemented comprehensively later (lines 67–107). Delete the
entire forEach that checks loader.use.filter and reassigns loader.use to the
filtered array so only the later SSR-specific loader adjustments remain,
ensuring no functional changes beyond removing the redundant duplicate.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against undefined when detecting css-loader.
Avoid potential TypeError if testValue is not set.
Apply this diff:
- return testValue.includes('css-loader');
+ return typeof testValue === 'string' && testValue.includes('css-loader');
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const cssLoader = rule.use.find((item) => { | |
let testValue; | |
if (typeof item === 'string') { | |
testValue = item; | |
} else if (typeof item.loader === 'string') { | |
testValue = item.loader; | |
} | |
return testValue.includes('css-loader'); | |
}); | |
if (cssLoader && cssLoader.options && cssLoader.options.modules) { | |
const cssLoader = rule.use.find((item) => { | |
let testValue; | |
if (typeof item === 'string') { | |
testValue = item; | |
} else if (typeof item.loader === 'string') { | |
testValue = item.loader; | |
} | |
return typeof testValue === 'string' && testValue.includes('css-loader'); | |
}); | |
if (cssLoader && cssLoader.options && cssLoader.options.modules) { |
🤖 Prompt for AI Agents
In config/rspack/serverRspackConfig.js around lines 84 to 95, the detection of
css-loader calls testValue.includes(...) without ensuring testValue is defined
which can throw a TypeError; update the predicate to guard testValue (e.g. check
testValue is a non-empty string before calling includes or use optional
chaining) so the find callback returns false when testValue is undefined, and
keep the subsequent check for cssLoader.options as-is.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add null safety check for rule.use.options.
When rule.use
is an object (not an array), accessing rule.use.options.emitFile
without verifying that options
exists could cause a runtime error if a loader doesn't define options.
Apply this diff:
// Skip writing image files during SSR by setting emitFile to false
} else if (rule.use && (rule.use.loader === 'url-loader' || rule.use.loader === 'file-loader')) {
- rule.use.options.emitFile = false;
+ if (rule.use.options) {
+ rule.use.options.emitFile = false;
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
} else if (rule.use && (rule.use.loader === 'url-loader' || rule.use.loader === 'file-loader')) { | |
rule.use.options.emitFile = false; | |
} | |
// Skip writing image files during SSR by setting emitFile to false | |
} else if (rule.use && (rule.use.loader === 'url-loader' || rule.use.loader === 'file-loader')) { | |
if (rule.use.options) { | |
rule.use.options.emitFile = false; | |
} | |
} |
🧰 Tools
🪛 ESLint
[error] 105-105: Assignment to property of function parameter 'rule'.
(no-param-reassign)
🤖 Prompt for AI Agents
In config/rspack/serverRspackConfig.js around lines 104 to 106, the code sets
rule.use.options.emitFile without ensuring rule.use.options exists; update the
branch handling object-style rule.use to first ensure rule.use.options is
defined (e.g. if (!rule.use.options) rule.use.options = {}), then set
rule.use.options.emitFile = false so you avoid runtime errors when options is
undefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Eliminate code duplication with
defaultMessages
.The English translations here duplicate
defaultMessages
fromclient/app/libs/i18n/default.js
. This violates the DRY principle and creates a maintenance burden—updating English text requires changes in two places.Apply this refactor to import and reuse
defaultMessages
:📝 Committable suggestion
🤖 Prompt for AI Agents