Skip to content

Commit 0d8d75a

Browse files
justin808claude
andcommitted
Address PR review feedback: fix bugs, security, and config issues
- Replace hardcoded localhost URL with RAILS_INTERNAL_URL env variable - Add response.ok check to prevent silent fetch failures - Guard 800ms demo delay to non-production environments - Restrict sanitize-html img tag to explicit allowed attributes/schemes - Clear useClientCache on each compilation for correct watch mode - Remove incorrect 'use client' from server-only files - Fix import/order lint violation in rsc-client-components - Gate trace option to development environment only - Remove duplicate RspackRscPlugin from server config (RSC-only) - Fix url-loader/file-loader guard to use .includes() matching - Pass RSC config to envSpecific callback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 05cb2bc commit 0d8d75a

9 files changed

Lines changed: 30 additions & 17 deletions

File tree

app/views/pages/server_components.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
<%= react_component("ServerComponentsPage",
33
prerender: false,
44
auto_load_bundle: false,
5-
trace: true,
5+
trace: Rails.env.development?,
66
id: "ServerComponentsPage-react-component-0") %>

client/app/bundles/comments/startup/RouterApp/ror_components/RouterApp.server.jsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
'use client';
2-
31
// Compare to ./RouterApp.client.jsx
42
import { Provider } from 'react-redux';
53
import React from 'react';

client/app/bundles/comments/startup/serverRegistration.jsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
'use client';
2-
31
// Example of React + Redux
42
import ReactOnRails from 'react-on-rails-pro';
53

client/app/bundles/server-components/components/CommentsFeed.jsx

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,19 @@ const marked = new Marked();
1212
marked.use(gfmHeadingId());
1313

1414
async function CommentsFeed() {
15-
// Simulate network latency to demonstrate streaming
16-
// eslint-disable-next-line no-promise-executor-return
17-
await new Promise((resolve) => setTimeout(resolve, 800));
15+
// Simulate network latency to demonstrate Suspense streaming (development only)
16+
if (process.env.NODE_ENV !== 'production') {
17+
await new Promise((resolve) => {
18+
setTimeout(resolve, 800);
19+
});
20+
}
1821

1922
// Fetch comments directly from the Rails API — no client-side fetch needed
20-
const response = await fetch('http://localhost:3000/comments.json');
23+
const baseUrl = process.env.RAILS_INTERNAL_URL || 'http://localhost:3000';
24+
const response = await fetch(`${baseUrl}/comments.json`);
25+
if (!response.ok) {
26+
throw new Error(`Failed to fetch comments: ${response.status} ${response.statusText}`);
27+
}
2128
const comments = await response.json();
2229

2330
// Use lodash to process (stays on server)
@@ -45,6 +52,11 @@ async function CommentsFeed() {
4552
const rawHtml = marked.parse(comment.text || '');
4653
const safeHtml = sanitizeHtml(rawHtml, {
4754
allowedTags: sanitizeHtml.defaults.allowedTags.concat(['img']),
55+
allowedAttributes: {
56+
...sanitizeHtml.defaults.allowedAttributes,
57+
img: ['src', 'alt', 'title', 'width', 'height'],
58+
},
59+
allowedSchemes: ['https', 'http', 'data'],
4860
});
4961

5062
return (

client/app/packs/rsc-client-components.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
// Components with 'use client' that are used in server components must be
55
// available in a client bundle chunk so the React flight client can load them.
66

7-
import TogglePanel from '../bundles/server-components/components/TogglePanel';
87
import ReactOnRails from 'react-on-rails-pro';
8+
import TogglePanel from '../bundles/server-components/components/TogglePanel';
99

1010
// Register as a standard component so it's bundled in a client-accessible chunk
1111
ReactOnRails.register({ TogglePanel });

config/webpack/rscWebpackConfig.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,11 @@ const configureRsc = () => {
6262
if (cssLoader?.options?.modules) {
6363
cssLoader.options.modules = { ...cssLoader.options.modules, exportOnlyLocals: true };
6464
}
65-
} else if (rule.use && (rule.use.loader === 'url-loader' || rule.use.loader === 'file-loader')) {
66-
rule.use.options.emitFile = false;
65+
} else if (
66+
rule.use?.loader
67+
&& (rule.use.loader.includes('url-loader') || rule.use.loader.includes('file-loader'))
68+
) {
69+
rule.use.options = { ...(rule.use.options || {}), emitFile: false };
6770
}
6871
});
6972

config/webpack/rspackRscPlugin.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ class RspackRscPlugin {
4848
}
4949

5050
apply(compiler) {
51+
// Clear cache on each compilation so watch-mode picks up 'use client' changes
52+
compiler.hooks.thisCompilation.tap('RspackRscPlugin-ClearCache', () => {
53+
useClientCache.clear();
54+
});
55+
5156
compiler.hooks.thisCompilation.tap('RspackRscPlugin', (compilation) => {
5257
compilation.hooks.processAssets.tap(
5358
{

config/webpack/serverWebpackConfig.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ const path = require('path');
55
const { config } = require('shakapacker');
66
const commonWebpackConfig = require('./commonWebpackConfig');
77
const { getBundler } = require('./bundlerUtils');
8-
const { RspackRscPlugin } = require('./rspackRscPlugin');
98

109
/**
1110
* Extract a specific loader from a webpack rule's use array.
@@ -172,9 +171,6 @@ const configureServer = () => {
172171
'react-dom/server.browser.js$': 'react-dom/server.node.js',
173172
};
174173

175-
// RSC: Generate react-server-client-manifest.json for SSR component resolution
176-
serverWebpackConfig.plugins.push(new RspackRscPlugin({ isServer: true }));
177-
178174
return serverWebpackConfig;
179175
};
180176

config/webpack/webpackConfig.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ const webpackConfig = (envSpecific) => {
2222
result = serverConfig;
2323
} else if (process.env.RSC_BUNDLE_ONLY) {
2424
const rscConfig = rscWebpackConfig();
25+
if (envSpecific) envSpecific(null, null, rscConfig);
2526
// eslint-disable-next-line no-console
2627
console.log('[React on Rails] Creating only the RSC bundle.');
2728
result = rscConfig;
2829
} else {
2930
const clientConfig = clientWebpackConfig();
3031
const serverConfig = serverWebpackConfig();
3132
const rscConfig = rscWebpackConfig();
32-
if (envSpecific) envSpecific(clientConfig, serverConfig);
33+
if (envSpecific) envSpecific(clientConfig, serverConfig, rscConfig);
3334
// eslint-disable-next-line no-console
3435
console.log('[React on Rails] Creating client, server, and RSC bundles.');
3536
result = [clientConfig, serverConfig, rscConfig];

0 commit comments

Comments
 (0)