-
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
base: master
Are you sure you want to change the base?
Changes from all commits
a951f4c
879d171
087ec70
5d85f15
3fe61f0
fbc5781
76921b8
012b0b7
1685fb4
28014b2
3da3dfc
71b934a
752919b
4c761bb
431a8ee
2e03f56
5f92988
0ab9eac
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,44 @@ | ||
/** | ||
* Bundler utilities for automatic Webpack/Rspack detection. | ||
* | ||
* Shakapacker 9.1+ supports both Webpack and Rspack as bundlers. | ||
* The bundler is selected via config/shakapacker.yml: | ||
* assets_bundler: webpack # or 'rspack' | ||
*/ | ||
|
||
const { config } = require('shakapacker'); | ||
|
||
/** | ||
* Gets the appropriate bundler module based on shakapacker.yml configuration. | ||
* | ||
* @returns {Object} Either webpack or @rspack/core module | ||
*/ | ||
const getBundler = () => { | ||
return config.assets_bundler === 'rspack' | ||
? require('@rspack/core') | ||
: require('webpack'); | ||
}; | ||
|
||
/** | ||
* Checks if the current bundler is Rspack. | ||
* | ||
* @returns {boolean} True if using Rspack, false if using Webpack | ||
*/ | ||
const isRspack = () => config.assets_bundler === 'rspack'; | ||
|
||
/** | ||
* Gets the appropriate CSS extraction plugin for the current bundler. | ||
* | ||
* @returns {Object} Either mini-css-extract-plugin (Webpack) or CssExtractRspackPlugin (Rspack) | ||
*/ | ||
const getCssExtractPlugin = () => { | ||
return isRspack() | ||
? getBundler().CssExtractRspackPlugin | ||
: require('mini-css-extract-plugin'); | ||
}; | ||
|
||
module.exports = { | ||
getBundler, | ||
isRspack, | ||
getCssExtractPlugin, | ||
}; |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,14 +4,14 @@ | |||||||||||||
// Common configuration applying to client and server configuration | ||||||||||||||
const { generateWebpackConfig, merge } = require('shakapacker'); | ||||||||||||||
|
||||||||||||||
const baseClientWebpackConfig = generateWebpackConfig(); | ||||||||||||||
const commonOptions = { | ||||||||||||||
resolve: { | ||||||||||||||
extensions: ['.css', '.ts', '.tsx'], | ||||||||||||||
// Add .bs.js extension for ReScript-compiled modules | ||||||||||||||
extensions: ['.css', '.ts', '.tsx', '.bs.js'], | ||||||||||||||
}, | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
// add sass resource loader | ||||||||||||||
// Sass resource loader config - globally imports app variables | ||||||||||||||
const sassLoaderConfig = { | ||||||||||||||
loader: 'sass-resources-loader', | ||||||||||||||
options: { | ||||||||||||||
|
@@ -20,56 +20,87 @@ const sassLoaderConfig = { | |||||||||||||
}; | ||||||||||||||
|
||||||||||||||
const ignoreWarningsConfig = { | ||||||||||||||
// React 19 uses react-dom/client but not all deps have migrated yet | ||||||||||||||
ignoreWarnings: [/Module not found: Error: Can't resolve 'react-dom\/client'/], | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
const scssConfigIndex = baseClientWebpackConfig.module.rules.findIndex((config) => | ||||||||||||||
'.scss'.match(config.test) && config.use, | ||||||||||||||
); | ||||||||||||||
/** | ||||||||||||||
* Generates the common webpack/rspack configuration used by both client and server bundles. | ||||||||||||||
* | ||||||||||||||
* IMPORTANT: This function calls generateWebpackConfig() fresh on each invocation, so mutations | ||||||||||||||
* to the returned config are safe and won't affect other builds. The config is regenerated | ||||||||||||||
* for each build (client, server, etc.). | ||||||||||||||
* | ||||||||||||||
* Key customizations: | ||||||||||||||
* - CSS Modules: Configured for default exports (namedExport: false) for backward compatibility | ||||||||||||||
* - Sass: Configured with modern API and global variable imports | ||||||||||||||
* - ReScript: Added .bs.js to resolve extensions | ||||||||||||||
* | ||||||||||||||
* @returns {Object} Webpack/Rspack configuration object (auto-detected based on shakapacker.yml) | ||||||||||||||
*/ | ||||||||||||||
const commonWebpackConfig = () => { | ||||||||||||||
// Generate fresh config - safe to mutate since it's a new object each time | ||||||||||||||
const baseWebpackConfig = generateWebpackConfig(); | ||||||||||||||
|
||||||||||||||
if (scssConfigIndex === -1) { | ||||||||||||||
console.warn('No SCSS rule with use array found in webpack config'); | ||||||||||||||
} else { | ||||||||||||||
// Configure sass-loader to use the modern API | ||||||||||||||
const scssRule = baseClientWebpackConfig.module.rules[scssConfigIndex]; | ||||||||||||||
const sassLoaderIndex = scssRule.use.findIndex((loader) => { | ||||||||||||||
if (typeof loader === 'string') { | ||||||||||||||
return loader.includes('sass-loader'); | ||||||||||||||
// Fix CSS Modules to use default exports for backward compatibility | ||||||||||||||
// Shakapacker 9 changed default to namedExport: true, breaking existing imports like: | ||||||||||||||
// import css from './file.module.scss' | ||||||||||||||
// This ensures css is an object with properties, not undefined | ||||||||||||||
baseWebpackConfig.module.rules.forEach((rule) => { | ||||||||||||||
if (rule.use && Array.isArray(rule.use)) { | ||||||||||||||
const cssLoader = rule.use.find((loader) => { | ||||||||||||||
const loaderName = typeof loader === 'string' ? loader : loader?.loader; | ||||||||||||||
return loaderName?.includes('css-loader'); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
if (cssLoader?.options?.modules) { | ||||||||||||||
cssLoader.options.modules.namedExport = false; | ||||||||||||||
cssLoader.options.modules.exportLocalsConvention = 'camelCase'; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
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'; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
const scssConfigIndex = baseWebpackConfig.module.rules.findIndex((config) => | ||||||||||||||
'.scss'.match(config.test) && config.use, | ||||||||||||||
); | ||||||||||||||
Comment on lines
+63
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Incorrect regex matching logic (regression). The expression Note: A previous review comment indicated this was fixed in commit 431a8ee, but the current code still contains the bug, suggesting either a regression or the fix wasn't applied. Apply this diff to fix the logic: - const scssConfigIndex = baseWebpackConfig.module.rules.findIndex((config) =>
- '.scss'.match(config.test) && config.use,
- );
+ const scssConfigIndex = baseWebpackConfig.module.rules.findIndex(
+ (config) => config.test?.test('.scss') && config.use,
+ ); 📝 Committable suggestion
Suggested change
🧰 Tools🪛 ESLint[error] 63-64: Replace (prettier/prettier) 🤖 Prompt for AI Agents
|
||||||||||||||
|
||||||||||||||
// 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 (scssConfigIndex === -1) { | ||||||||||||||
console.warn('No SCSS rule with use array found in webpack config'); | ||||||||||||||
// Not throwing error since config might work without SCSS | ||||||||||||||
} else { | ||||||||||||||
// Configure sass-loader to use the modern API | ||||||||||||||
const scssRule = baseWebpackConfig.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 (cssLoader?.options?.modules?.namedExport) { | ||||||||||||||
cssLoader.options.modules.exportLocalsConvention = 'camelCaseOnly'; | ||||||||||||||
} | ||||||||||||||
if (sassLoaderIndex !== -1) { | ||||||||||||||
const sassLoader = scssRule.use[sassLoaderIndex]; | ||||||||||||||
if (typeof sassLoader === 'string') { | ||||||||||||||
scssRule.use[sassLoaderIndex] = { | ||||||||||||||
loader: sassLoader, | ||||||||||||||
options: { | ||||||||||||||
// Use modern API for better performance and to support sass-resources-loader | ||||||||||||||
// The modern API uses the Sass JavaScript API instead of the legacy Node API | ||||||||||||||
api: 'modern' | ||||||||||||||
} | ||||||||||||||
}; | ||||||||||||||
} else { | ||||||||||||||
sassLoader.options = sassLoader.options || {}; | ||||||||||||||
// Use modern API for better performance and to support sass-resources-loader | ||||||||||||||
// The modern API uses the Sass JavaScript API instead of the legacy Node API | ||||||||||||||
sassLoader.options.api = 'modern'; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
baseClientWebpackConfig.module.rules[scssConfigIndex].use.push(sassLoaderConfig); | ||||||||||||||
} | ||||||||||||||
baseWebpackConfig.module.rules[scssConfigIndex].use.push(sassLoaderConfig); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Copy the object using merge b/c the baseClientWebpackConfig and commonOptions are mutable globals | ||||||||||||||
const commonWebpackConfig = () => merge({}, baseClientWebpackConfig, commonOptions, ignoreWarningsConfig); | ||||||||||||||
return merge({}, baseWebpackConfig, commonOptions, ignoreWarningsConfig); | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
module.exports = commonWebpackConfig; | ||||||||||||||
|
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.
Fix the Table of Contents anchor
Renaming the heading to “Webpack and Rspack” means the TOC link
[Webpack](#webpack)
no longer resolves. Please update the TOC entry to match the new slug (#webpack-and-rspack
) to avoid a broken navigation link.🤖 Prompt for AI Agents