Add comprehensive documentation and address code review feedback

Documentation:
- Added JSDoc comments to commonWebpackConfig() and configureServer()
- Created patches/README.md explaining patch necessity and maintenance
- Updated variable names for clarity (baseClientRspackConfig -> baseWebpackConfig)
- Added inline comments explaining critical fixes

Code Quality:
- Added comment about safe mutation pattern (fresh config each call)
- Added clarifying comments for console.warn (not throwing error)
- Improved CSS modules fix comments with examples
- Added bundler auto-detection explanations

Upstream Contributions:
- Filed issue with @glennsl/rescript-json-combinators: #9
- Updated patches/README.md with issue link
- Comprehensively updated react_on_rails issue #1863 with ALL migration challenges

Key documentation improvements:
- Timeline of 11 commits and issues resolved
- Root cause analysis for each problem
- Complete code examples for each fix
- Impact assessment (3 days → 2-3 commits with docs)

This addresses all code review feedback from the PR review.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: justin808

config/webpack/commonWebpackConfig.js

@@ -6,11 +6,12 @@ const { generateWebpackConfig, merge } = require('shakapacker');
 
 const commonOptions = {
   resolve: {
+    // 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: {
@@ -19,16 +20,33 @@ 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'/],
 };
 
-// Copy the object using merge b/c the baseClientWebpackConfig and commonOptions are mutable globals
+/**
+ * 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 = () => {
-  const baseClientWebpackConfig = generateWebpackConfig();
+  // Generate fresh config - safe to mutate since it's a new object each time
+  const baseWebpackConfig = generateWebpackConfig();
 
-  // Fix all CSS-related loaders to use default exports instead of named exports
-  // Shakapacker 9 defaults to namedExport: true, but existing code expects default exports
-  baseClientWebpackConfig.module.rules.forEach((rule) => {
+  // 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;
@@ -42,15 +60,16 @@ const commonWebpackConfig = () => {
     }
   });
 
-  const scssConfigIndex = baseClientWebpackConfig.module.rules.findIndex((config) =>
+  const scssConfigIndex = baseWebpackConfig.module.rules.findIndex((config) =>
     '.scss'.match(config.test) && config.use,
   );
 
   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 = baseClientWebpackConfig.module.rules[scssConfigIndex];
+    const scssRule = baseWebpackConfig.module.rules[scssConfigIndex];
     const sassLoaderIndex = scssRule.use.findIndex((loader) => {
       if (typeof loader === 'string') {
         return loader.includes('sass-loader');
@@ -73,10 +92,10 @@ const commonWebpackConfig = () => {
       }
     }
 
-    baseClientWebpackConfig.module.rules[scssConfigIndex].use.push(sassLoaderConfig);
+    baseWebpackConfig.module.rules[scssConfigIndex].use.push(sassLoaderConfig);
   }
 
-  return merge({}, baseClientWebpackConfig, commonOptions, ignoreWarningsConfig);
+  return merge({}, baseWebpackConfig, commonOptions, ignoreWarningsConfig);
 };
 
 module.exports = commonWebpackConfig;

config/webpack/serverWebpackConfig.js

@@ -6,10 +6,27 @@ const { config } = require('shakapacker');
 const commonWebpackConfig = require('./commonWebpackConfig');
 
 // Auto-detect bundler from shakapacker config and load the appropriate library
+// This allows the same config to work with both Webpack and Rspack
 const bundler = config.assets_bundler === 'rspack'
   ? require('@rspack/core')
   : require('webpack');
 
+/**
+ * Generates the server-side rendering (SSR) bundle configuration.
+ *
+ * This creates a separate bundle optimized for server-side rendering:
+ * - Single chunk (no code splitting for Node.js execution)
+ * - CSS extraction disabled (uses exportOnlyLocals for class name mapping)
+ * - No asset hashing (not served directly to clients)
+ * - Outputs to ssr-generated/ directory
+ *
+ * Key differences from client config:
+ * - Removes CSS extraction loaders (mini-css-extract-plugin/CssExtractRspackPlugin)
+ * - Preserves CSS Modules configuration but adds exportOnlyLocals: true
+ * - Disables optimization/minification for faster builds and better debugging
+ *
+ * @returns {Object} Webpack/Rspack configuration object for server bundle
+ */
 const configureServer = () => {
   // We need to use "merge" because the clientConfigObject, EVEN after running
   // toWebpackConfig() is a mutable GLOBAL. Thus any changes, like modifying the

patches/README.md

@@ -0,0 +1,52 @@
+# Patches
+
+This directory contains patches applied to npm packages using [patch-package](https://github.com/ds300/patch-package).
+
+## Why Patches?
+
+Patches are used when npm packages need modifications that haven't been released upstream yet, or when quick fixes are needed for compatibility issues.
+
+## Current Patches
+
+### @glennsl/rescript-json-combinators+1.4.0.patch
+
+**Issue**: This package ships with only ReScript source files (`.res`), not compiled JavaScript files (`.bs.js`). Its `bsconfig.json` lacks the `package-specs` configuration needed to generate compiled output.
+
+**Impact**: Without this patch, Rspack/Webpack cannot resolve imports like:
+```javascript
+import * as Json from "@glennsl/rescript-json-combinators/src/Json.bs.js";
+```
+
+**What the patch does**:
+1. Removes reference to non-existent `examples` directory
+2. Adds `package-specs` configuration for ES module output
+3. Adds `suffix: ".bs.js"` to generate `.bs.js` files
+
+**When applied**: Automatically during `yarn install` via the `postinstall` script
+
+**Upstream status**:
+- Opened issue: https://github.com/glennsl/rescript-json-combinators/issues/9
+- This is a common pattern for in-source builds with ReScript
+- May not be accepted upstream if author prefers source-only distribution
+
+**TODO**: Check if patch is still needed when upgrading `@glennsl/rescript-json-combinators`
+
+## How Patches Work
+
+1. **Creation**: When you modify a package in `node_modules/`, run:
+   ```bash
+   npx patch-package package-name
+   ```
+
+2. **Application**: Patches are automatically applied after `yarn install` via the `postinstall` script in `package.json`
+
+3. **Updating**: If the package is updated and the patch fails, you'll need to either:
+   - Regenerate the patch with the new version
+   - Remove the patch if it's no longer needed
+   - Update the patch manually
+
+## Maintenance
+
+- **Before upgrading patched packages**: Check if the patch is still necessary
+- **If patch fails to apply**: The build will fail with a clear error message
+- **Review patches regularly**: Consider contributing fixes upstream to reduce maintenance burden