Skip to content

Commit 8032762

Browse files
ihabadhamclaude
andcommitted
Trim over-verbose comments across Sub-PR 2 files
Per the repo's CLAUDE.md guidance ("default to writing no comments; only when the WHY is non-obvious"), stripping comments that explain WHAT the code does or reference the current-PR context. Kept (non-obvious WHYs): the `additionalContext: { URL, AbortController }` reason (react-router-dom NavLink url-is-not-defined), the CI worker-count cap reason, `clientReferences` scoping rationale, `libraryTarget: 'commonjs2'` requirement for the renderer, `target: 'node'` fix for SSR-breaking libs, the renderer-fallback-disabled reason, the renderer-password-must-match pointer, the bundler-utils dual-support blurb, and the shakapacker.yml tactical-reversal tag. Dropped (WHAT explanations, redundant with identifiers): JSDoc blocks describing `configureServer` and `extractLoader`, per-config-key descriptions in the renderer launcher, Procfile.dev process-purpose comment, verbose `.env.example` prose, initializer multi-line explanations of each field. No code changes — comments only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8673a23 commit 8032762

6 files changed

Lines changed: 29 additions & 114 deletions

File tree

.env.example

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,11 @@
1-
# React on Rails Pro license — required in production.
2-
# Issue a license at https://pro.reactonrails.com/ and paste the JWT token below.
3-
# Not required in development or test (the Pro engine logs an info notice and
4-
# continues). Don't commit the real token — copy this file to `.env`, which is
5-
# gitignored.
1+
# React on Rails Pro license (JWT token from https://pro.reactonrails.com/).
2+
# Required in production; optional in development/test.
63
REACT_ON_RAILS_PRO_LICENSE=
74

8-
# Shared secret between the Rails app and the Node renderer. Must match
9-
# config.renderer_password in config/initializers/react_on_rails_pro.rb.
10-
# In production, set this to a strong random value via your secret store.
11-
# In development, both the renderer and the initializer default to
12-
# `local-dev-renderer-password` so `bin/dev` works without any setup.
5+
# Shared secret between Rails and the Node renderer. Must match on both sides.
6+
# Dev/test default to `local-dev-renderer-password` if unset; production must
7+
# set this explicitly (Pro raises at boot otherwise).
138
RENDERER_PASSWORD=
149

15-
# Node renderer HTTP endpoint. Defaults to http://localhost:3800 in
16-
# development (where Procfile.dev runs `node renderer/node-renderer.js`).
17-
# Override in production to point at the deployed renderer host.
10+
# Node renderer endpoint. Defaults to http://localhost:3800.
1811
# REACT_RENDERER_URL=http://localhost:3800

Procfile.dev

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,4 @@ rails: bundle exec thrust bin/rails server -p 3000
1212
wp-client: RAILS_ENV=development NODE_ENV=development bin/shakapacker-dev-server
1313
# Server webpack watcher for SSR bundle
1414
wp-server: SERVER_BUNDLE_ONLY=yes bin/shakapacker --watch
15-
# Pro Node renderer — executes the server bundle for SSR (port 3800)
1615
node-renderer: NODE_ENV=development node renderer/node-renderer.js
Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,15 @@
11
# frozen_string_literal: true
22

33
# See https://reactonrails.com/docs/configuration/configuration-pro
4-
# Production license: set REACT_ON_RAILS_PRO_LICENSE to the JWT token from
5-
# https://pro.reactonrails.com/. Development and test environments don't
6-
# require a license — the Pro engine logs an info-level notice only.
4+
# License: Set REACT_ON_RAILS_PRO_LICENSE environment variable
75
ReactOnRailsPro.configure do |config|
8-
# Route all server rendering through the Pro Node renderer (port 3800).
9-
# Falling back to ExecJS is disabled so any renderer issue surfaces loudly
10-
# instead of silently degrading.
116
config.server_renderer = "NodeRenderer"
7+
# Raise loudly when the renderer is unavailable instead of silently
8+
# degrading to ExecJS (default is true).
129
config.renderer_use_fallback_exec_js = false
1310

14-
# Renderer HTTP endpoint. The Node renderer listens on localhost:3800 in
15-
# development; override REACT_RENDERER_URL in production to point at the
16-
# deployed renderer host.
1711
config.renderer_url = ENV.fetch("REACT_RENDERER_URL", "http://localhost:3800")
1812

19-
# Shared secret for renderer authentication. Must match renderer/node-renderer.js.
20-
# In production-like envs, Pro's configuration.rb raises at boot if this is
21-
# blank and RENDERER_PASSWORD isn't in the environment — so the dev fallback
22-
# below never reaches prod as long as RENDERER_PASSWORD is set there.
13+
# Must match the password in renderer/node-renderer.js.
2314
config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "local-dev-renderer-password")
2415
end

config/webpack/bundlerUtils.js

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,11 @@
1-
/**
2-
* Bundler utilities. Returns the active bundler module based on shakapacker.yml's
3-
* `assets_bundler` setting. Supports both webpack and rspack so this project can
4-
* switch between them without touching every config file.
5-
*/
1+
// Returns the active bundler module per shakapacker.yml's `assets_bundler`.
2+
// Supports both webpack and rspack so this project can switch between them
3+
// without touching every config file.
64

75
const { config } = require('shakapacker');
86

97
let _cachedBundler = null;
108

11-
/**
12-
* Gets the bundler module for the current build.
13-
*
14-
* @returns {Object} webpack or @rspack/core module
15-
*/
169
const getBundler = () => {
1710
if (_cachedBundler) {
1811
return _cachedBundler;
@@ -23,20 +16,10 @@ const getBundler = () => {
2316
return _cachedBundler;
2417
};
2518

26-
/**
27-
* Checks whether the configured bundler is Rspack.
28-
*
29-
* @returns {boolean} True when assets_bundler is rspack
30-
*/
3119
const isRspack = () => config.assets_bundler === 'rspack';
3220

33-
/**
34-
* Gets the CSS extraction plugin. Only meaningful on rspack — webpack projects
35-
* use mini-css-extract-plugin directly via shakapacker's generated config.
36-
*
37-
* @returns {Object} CssExtractRspackPlugin
38-
* @throws {Error} If assets_bundler is not rspack
39-
*/
21+
// Only meaningful on rspack — webpack projects use mini-css-extract-plugin
22+
// via shakapacker's generated config.
4023
const getCssExtractPlugin = () => {
4124
if (!isRspack()) {
4225
throw new Error(

config/webpack/serverWebpackConfig.js

Lines changed: 9 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,6 @@ const { RSCWebpackPlugin } = require('react-on-rails-rsc/WebpackPlugin');
88
const commonWebpackConfig = require('./commonWebpackConfig');
99
const { getBundler } = require('./bundlerUtils');
1010

11-
/**
12-
* Locates a loader in a rule's `use` array by loader name substring.
13-
*
14-
* Exposed on the module exports so rscWebpackConfig.js can call it when
15-
* deriving the RSC bundle from serverWebpackConfig(true). Matches the
16-
* `extractLoader` helper used in the Pro dummy and marketplace references.
17-
*
18-
* @param {Object} rule Webpack module rule with a `use` array
19-
* @param {string} loaderName Substring to match against each loader's name
20-
* @returns {Object|string|undefined} Matching loader entry, or undefined
21-
*/
2211
function extractLoader(rule, loaderName) {
2312
if (!Array.isArray(rule.use)) return undefined;
2413
return rule.use.find((item) => {
@@ -27,26 +16,6 @@ function extractLoader(rule, loaderName) {
2716
});
2817
}
2918

30-
/**
31-
* Generates the server-side rendering (SSR) bundle configuration for Pro's
32-
* Node renderer.
33-
*
34-
* Key Pro-specific settings (applied after shakapacker's generated config):
35-
* - target: 'node' + node: false — required for Node execution in the renderer
36-
* - libraryTarget: 'commonjs2' — required so the renderer can `require()` the bundle
37-
* - RSCWebpackPlugin({ isServer: true }) — emits the server manifest used by
38-
* React Server Components (inert until RSC support is enabled in Sub-PR 3)
39-
* - babelLoader caller = { ssr: true } — lets Babel pick SSR-specific transforms
40-
* - CSS extraction disabled; css-loader switches to exportOnlyLocals for class
41-
* name mapping only
42-
*
43-
* The `rscBundle` arg exists so Sub-PR 3's rscWebpackConfig.js can derive the
44-
* RSC bundle from this same config — when true, the server-manifest plugin is
45-
* skipped (the RSC bundle emits the client manifest instead).
46-
*
47-
* @param {boolean} [rscBundle=false] True when called from rscWebpackConfig.js
48-
* @returns {Object} Webpack configuration object for the SSR bundle
49-
*/
5019
const configureServer = (rscBundle = false) => {
5120
const bundler = getBundler();
5221

@@ -56,7 +25,6 @@ const configureServer = (rscBundle = false) => {
5625
// Using webpack-merge into an empty object avoids this issue.
5726
const serverWebpackConfig = commonWebpackConfig();
5827

59-
// We just want the single server bundle entry
6028
const serverEntry = {
6129
'server-bundle': serverWebpackConfig.entry['server-bundle'],
6230
};
@@ -80,18 +48,15 @@ const configureServer = (rscBundle = false) => {
8048

8149
serverWebpackConfig.entry = serverEntry;
8250

83-
// No splitting of chunks for a server bundle
8451
serverWebpackConfig.optimization = {
8552
minimize: false,
8653
};
8754
serverWebpackConfig.plugins.unshift(new bundler.optimize.LimitChunkCountPlugin({ maxChunks: 1 }));
8855

8956
if (!rscBundle) {
90-
// Limit client-reference discovery to the app source directory. Without
91-
// `clientReferences`, the plugin may traverse into node_modules/ and hit
92-
// non-JS source files (e.g. .tsx that aren't configured for a loader),
93-
// and would re-scan nodes we don't care about. Matches the Pro dummy
94-
// pattern in react_on_rails_pro/spec/dummy/config/webpack/serverWebpackConfig.js.
57+
// Scope client-reference discovery to the app source dir. Without this,
58+
// the plugin can walk into node_modules and hit .tsx source files that
59+
// aren't configured for a loader. Matches the Pro dummy pattern.
9560
serverWebpackConfig.plugins.push(
9661
new RSCWebpackPlugin({
9762
isServer: true,
@@ -102,21 +67,16 @@ const configureServer = (rscBundle = false) => {
10267
);
10368
}
10469

105-
// Custom output for the server-bundle.
106-
// - libraryTarget: 'commonjs2' is required by the Pro Node renderer so it
107-
// can `require()` the evaluated bundle.
108-
// - No publicPath: the server bundle is loaded by the Node renderer via the
109-
// filesystem, never served over HTTP, so asset URLs would go unused.
70+
// libraryTarget: 'commonjs2' is required by the Pro Node renderer so it can
71+
// `require()` the evaluated bundle. No publicPath: the server bundle is
72+
// loaded from the filesystem, never served over HTTP.
11073
serverWebpackConfig.output = {
11174
filename: 'server-bundle.js',
11275
globalObject: 'this',
11376
libraryTarget: 'commonjs2',
11477
path: path.resolve(__dirname, '../../ssr-generated'),
11578
};
11679

117-
// Don't hash the server bundle b/c would conflict with the client manifest
118-
// And no need for CSS extraction plugins (webpack's MiniCssExtractPlugin or
119-
// rspack's CssExtractRspackPlugin).
12080
serverWebpackConfig.plugins = serverWebpackConfig.plugins.filter(
12181
(plugin) =>
12282
plugin.constructor.name !== 'WebpackAssetsManifest' &&
@@ -125,11 +85,6 @@ const configureServer = (rscBundle = false) => {
12585
plugin.constructor.name !== 'ForkTsCheckerWebpackPlugin',
12686
);
12787

128-
// Configure loader rules for SSR:
129-
// - strip CSS extraction and style-loader (client build handles CSS export)
130-
// - css-loader exportOnlyLocals: true (keep class name mapping only)
131-
// - babel-loader caller.ssr: true (Babel picks SSR-specific transforms)
132-
// - url/file-loader emitFile: false (don't duplicate image assets during SSR)
13388
serverWebpackConfig.module.rules.forEach((rule) => {
13489
if (Array.isArray(rule.use)) {
13590
rule.use = rule.use.filter((item) => {
@@ -161,16 +116,12 @@ const configureServer = (rscBundle = false) => {
161116
}
162117
});
163118

164-
// eval works well for the SSR bundle because it's the fastest and shows
165-
// lines in the server bundle which is good for debugging SSR.
166119
serverWebpackConfig.devtool = 'eval';
167120

168-
// Target 'node' so the bundle uses real CommonJS require() and Node globals
169-
// like __dirname. Avoids polyfills and fixes libraries like Emotion and
170-
// loadable-components that break under target: 'web' in SSR.
121+
// target: 'node' fixes SSR breakage in libraries (Emotion, loadable-components, etc.)
122+
// that don't behave under the default 'web' target. node: false disables the
123+
// polyfill shims that only matter when targeting 'web'.
171124
serverWebpackConfig.target = 'node';
172-
173-
// Disable Node.js polyfill shims — not needed when targeting Node.
174125
serverWebpackConfig.node = false;
175126

176127
return serverWebpackConfig;

renderer/node-renderer.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,19 @@ function parseIntegerEnv(name, defaultValue, { min, max = Number.MAX_SAFE_INTEGE
2929

3030
const config = {
3131
serverBundleCachePath: path.resolve(__dirname, '.node-renderer-bundles'),
32-
// Default to info per Pro docs (docs/oss/building-features/node-renderer/
33-
// container-deployment.md: "logLevel: 'info' // General renderer logs").
34-
// Override with RENDERER_LOG_LEVEL=debug when actively debugging.
3532
logLevel: process.env.RENDERER_LOG_LEVEL || 'info',
3633
password: rendererPassword,
3734
port: parseIntegerEnv('RENDERER_PORT', 3800, { min: 1, max: 65535 }),
3835
supportModules: true,
3936
workersCount: parseIntegerEnv('NODE_RENDERER_CONCURRENCY', 3, { min: 0 }),
40-
// Expose globals the renderer's VM sandbox doesn't auto-provide but that
41-
// downstream dependencies use during SSR (react-router-dom's NavLink calls
42-
// `new URL(...)` via encodeLocation; various libs use AbortController).
37+
// Expose globals the VM sandbox doesn't auto-provide but that downstream
38+
// deps rely on during SSR. Without URL, react-router-dom's NavLink throws
39+
// `ReferenceError: URL is not defined` via encodeLocation.
4340
additionalContext: { URL, AbortController },
4441
};
4542

46-
// Cap CI workers unless the caller has set NODE_RENDERER_CONCURRENCY explicitly.
43+
// CI hosts report more CPUs than allocated to the container; cap workers to
44+
// avoid oversubscribing memory.
4745
if (process.env.CI && process.env.NODE_RENDERER_CONCURRENCY == null) {
4846
config.workersCount = 2;
4947
}

0 commit comments

Comments
 (0)