Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Procfile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ rails: bundle exec thrust bin/rails server -p 3000
wp-client: RAILS_ENV=development NODE_ENV=development bin/shakapacker-dev-server
# Server webpack watcher for SSR bundle
wp-server: SERVER_BUNDLE_ONLY=yes bin/shakapacker --watch
# RSC webpack watcher for React Server Components bundle
wp-rsc: RSC_BUNDLE_ONLY=yes bin/shakapacker --watch
node-renderer: NODE_ENV=development node renderer/node-renderer.js
7 changes: 7 additions & 0 deletions app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

class PagesController < ApplicationController
include ReactOnRails::Controller
include ReactOnRailsPro::Stream
before_action :set_comments

def index
Expand Down Expand Up @@ -38,6 +39,12 @@ def simple; end

def rescript; end

def server_components
@server_components_comments = Comment.order(id: :desc).limit(10)
.as_json(only: %i[id author text created_at updated_at])
stream_view_containing_react_components(template: "/pages/server_components")
Comment on lines +42 to +45
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip unbounded comment preload in server components action

server_components already fetches the latest 10 comments, but this action still runs the controller-wide before_action :set_comments, which loads the full comments relation on every /server-components request and is never used here. As the comments table grows, that extra query/allocation becomes a real latency and memory hit for this page; scoping set_comments to the actions that need @comments (or skipping it for this action) avoids the regression.

Useful? React with 👍 / 👎.

end

private

def set_comments
Expand Down
6 changes: 6 additions & 0 deletions app/views/pages/server_components.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<%= stream_react_component("ServerComponentsPage",
props: { comments: @server_components_comments },
prerender: true,
auto_load_bundle: true,
trace: Rails.env.development?,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding the element id is fragile — it will silently create duplicate IDs if the helper is ever called more than once (e.g. if this partial is rendered in a layout that includes it twice, or in a test that renders the view multiple times). React on Rails generates a unique ID automatically when none is provided; consider removing this option and letting the helper handle it:

Suggested change
trace: Rails.env.development?,
trace: Rails.env.development?) %>

id: "ServerComponentsPage-react-component-0") %>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit id: is unusual here — React on Rails auto-generates stable IDs. Hardcoding "ServerComponentsPage-react-component-0" means a second react_component call on the same page (e.g. if this partial is ever included twice, or if a test helper renders it multiple times) would produce a duplicate HTML id, breaking DOM queries and accessibility tooling. Unless this specific ID is required by the Pro RSC runtime, dropping the parameter lets Rails generate it automatically.

Suggested change
id: "ServerComponentsPage-react-component-0") %>
<%= react_component("ServerComponentsPage",
prerender: false,
auto_load_bundle: true,
trace: Rails.env.development?) %>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react_on_rails generates stable, unique component IDs automatically. Hardcoding one here adds fragility (will conflict if the helper is called twice) and diverges from every other view in the app. Remove this option and let the framework manage the ID:

Suggested change
id: "ServerComponentsPage-react-component-0") %>
trace: Rails.env.development?) %>

Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

import React from 'react';
import PropTypes from 'prop-types';
import BaseComponent from 'libs/components/BaseComponent';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ function NavigationBar(props) {
Rescript
</a>
</li>
<li>
<a
className={navItemClassName(pathname === paths.SERVER_COMPONENTS_PATH)}
href={paths.SERVER_COMPONENTS_PATH}
>
RSC Demo
</a>
</li>
<li>
<a
className={navItemClassName(false)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

// eslint-disable-next-line max-classes-per-file
import React from 'react';
import request from 'axios';
Expand Down
1 change: 1 addition & 0 deletions client/app/bundles/comments/constants/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export const RESCRIPT_PATH = '/rescript';
export const SIMPLE_REACT_PATH = '/simple';
export const STIMULUS_PATH = '/stimulus';
export const RAILS_PATH = '/comments';
export const SERVER_COMPONENTS_PATH = '/server-components';
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

// Wrapper for ReScript component to work with react_on_rails auto-registration
// react_on_rails looks for components in ror_components/ subdirectories

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

import { Provider } from 'react-redux';
import React from 'react';
import ReactOnRails from 'react-on-rails-pro';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

// Top level component for client side.
// Compare this to the ./ServerApp.jsx file which is used for server side rendering.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

// Compare to ./RouterApp.server.jsx
import { Provider } from 'react-redux';
import React from 'react';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove 'use client' from server-only router entry

Marking this .server.jsx module as a client component is unsafe because it imports react-router-dom/server, and the client build is now configured to discover 'use client' files across the whole client/app tree via RSCWebpackPlugin (clientReferences with recursive: true). That means this server-only file can be pulled into client reference/manifest processing, which can introduce server-only dependencies into the client pipeline and fail or regress builds depending on resolver behavior.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A file named .server.jsx with 'use client' at the top is a contradiction that will confuse future contributors unfamiliar with this RSC setup. The .server suffix suggests a React Server Component, but this file is actually the SSR wrapper for the React Router client app (it runs in Node via the classic SSR path, not the RSC flight renderer).

Please add a short inline explanation, e.g.:

Suggested change
'use client';
'use client';
// NOTE: Despite the .server.jsx filename this is NOT a React Server Component.
// It is the classic SSR entry for React Router and uses Redux Provider, both
// of which are client-only APIs. 'use client' marks it as a client-reference
// boundary so the RSC compiler keeps it out of the server component graph.


// Compare to ./RouterApp.client.jsx
import { Provider } from 'react-redux';
import React from 'react';
Expand Down
84 changes: 84 additions & 0 deletions client/app/bundles/server-components/components/CommentsFeed.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import React from 'react';
import { Marked } from 'marked';
import { gfmHeadingId } from 'marked-gfm-heading-id';
import sanitizeHtml from 'sanitize-html';
import TogglePanel from './TogglePanel';

const marked = new Marked();
marked.use(gfmHeadingId());

// Default-on small delay so the surrounding <Suspense> fallback is visible
// in the demo. Set RSC_SUSPENSE_DEMO_DELAY=false to disable (CI / tests).
async function CommentsFeed({ comments = [] }) {
if (process.env.RSC_SUSPENSE_DEMO_DELAY !== 'false') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard is inverted — this delay fires in every environment unless RSC_SUSPENSE_DEMO_DELAY=false is explicitly set. A production deploy that forgets the env var will always add 800 ms.

Suggested change
if (process.env.RSC_SUSPENSE_DEMO_DELAY !== 'false') {
if (process.env.RSC_SUSPENSE_DEMO_DELAY === 'true') {

Or gate on NODE_ENV so it is always off in production regardless of the env var:

if (process.env.NODE_ENV !== 'production' && process.env.RSC_SUSPENSE_DEMO_DELAY !== 'false') {

await new Promise((resolve) => {
setTimeout(resolve, 800);
});
}

if (comments.length === 0) {
return (
<div className="bg-amber-50 border border-amber-200 rounded-lg p-6 text-center">
<p className="text-amber-700">
No comments yet. Add some comments from the{' '}
<a href="/" className="underline font-medium">
home page
</a>{' '}
to see them rendered here by server components.
</p>
</div>
);
}

return (
<div className="space-y-3">
{comments.map((comment) => {
// marked + sanitize-html (~200KB combined) stay server-side.
const rawHtml = marked.parse(comment.text || '');
const safeHtml = sanitizeHtml(rawHtml, {
allowedTags: sanitizeHtml.defaults.allowedTags.concat(['img']),
allowedAttributes: {
...sanitizeHtml.defaults.allowedAttributes,
img: ['src', 'alt', 'title', 'width', 'height'],
},
allowedSchemes: ['https', 'http'],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including 'http' allows unencrypted image URLs. On an HTTPS page, browsers block these as mixed content, and they can also serve as tracking pixels. Restrict to HTTPS only:

Suggested change
allowedSchemes: ['https', 'http'],
allowedSchemes: ['https'],

});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowedSchemes applies to every URL-type attribute (including img[src]). Allowing http exposes a few risks in production: mixed-content warnings in browsers, potential tracking pixels from plain-HTTP origins, and SSRF-adjacent issues if a proxy ever fetches these URLs server-side. Since the sanitisation already runs server-side, restricting to https only costs nothing at runtime.

Suggested change
});
allowedSchemes: ['https'],

Comment on lines +43 to +45
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 http scheme allows mixed-content images

allowedSchemes includes 'http', so user-submitted markdown with ![img](http://...) will produce <img src="http://..."> in the sanitized HTML. When the app is served over HTTPS (as it will be on any review/production deployment), browsers block those images as mixed-content and display broken image placeholders. Since this is a demo of server-side rendering, broken inline images would undermine the demo. Restricting to ['https'] is the safe default.

Suggested change
},
allowedSchemes: ['https', 'http'],
});
allowedSchemes: ['https'],


return (
<div
key={comment.id}
className="bg-white border border-slate-200 rounded-lg p-4 shadow-sm hover:shadow-md transition-shadow"
>
<div className="flex items-center justify-between mb-2">
<span className="font-semibold text-slate-800">{comment.author}</span>
<span className="text-xs text-slate-400">
{new Date(comment.created_at).toLocaleDateString('en-US', {
month: 'short',
day: 'numeric',
year: 'numeric',
hour: '2-digit',
minute: '2-digit',
})}
</span>
</div>
<TogglePanel title="Show rendered markdown">
{/* Content is sanitized via sanitize-html before rendering */}
{/* eslint-disable-next-line react/no-danger */}
<div
className="prose prose-sm prose-slate max-w-none"
dangerouslySetInnerHTML={{ __html: safeHtml }}
/>
</TogglePanel>
<p className="text-slate-600 text-sm mt-1">{comment.text}</p>
</div>
);
})}
<p className="text-xs text-slate-400 text-center pt-2">
{comments.length} comment{comments.length !== 1 ? 's' : ''} rendered on the server using{' '}
<code>marked</code> + <code>sanitize-html</code> (never sent to browser)
</p>
</div>
);
}

export default CommentsFeed;
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
'use client';

import React, { useState } from 'react';
import { ErrorBoundary } from 'react-error-boundary';
import RSCRoute from 'react-on-rails-pro/RSCRoute';
import { useRSC } from 'react-on-rails-pro/RSCProvider';

const LiveActivityRefresher = () => {
const [refreshKey, setRefreshKey] = useState(0);
const [simulateError, setSimulateError] = useState(false);
const { refetchComponent } = useRSC();

const handleRefresh = () => {
setSimulateError(false);
setRefreshKey((k) => k + 1);
};

const handleSimulateError = () => {
setSimulateError(true);
setRefreshKey((k) => k + 1);
};

// refetchComponent primes the cache with corrected props before resetting
// the boundary, so the post-reset render hits cache instead of re-fetching.
const buildRetry = (resetErrorBoundary) => () => {
const newKey = refreshKey + 1;
setSimulateError(false);
setRefreshKey(newKey);
refetchComponent('LiveActivity', { simulateError: false, refreshKey: newKey })
// eslint-disable-next-line no-console
.catch((err) => console.error('Retry refetch failed:', err))
.finally(() => resetErrorBoundary());
};

return (
<div className="space-y-3">
<div className="flex items-center gap-2">
<button
type="button"
onClick={handleRefresh}
className="px-3 py-1.5 text-sm bg-indigo-600 text-white rounded hover:bg-indigo-700"
>
Refresh
</button>
<button
type="button"
onClick={handleSimulateError}
className="px-3 py-1.5 text-sm bg-amber-100 text-amber-800 border border-amber-300 rounded hover:bg-amber-200"
>
Simulate Error
</button>
<span className="text-xs text-slate-500 ml-2">Refresh count: {refreshKey}</span>
</div>
<ErrorBoundary
fallbackRender={({ error, resetErrorBoundary }) => (
<div className="bg-rose-50 border border-rose-200 rounded-lg p-4">
<p className="text-rose-700 font-semibold mb-1">Server component fetch failed</p>
<p className="text-rose-600 text-sm font-mono mb-3">{error.message}</p>
<button
type="button"
onClick={buildRetry(resetErrorBoundary)}
className="px-3 py-1.5 text-sm bg-rose-600 text-white rounded hover:bg-rose-700"
>
Retry
</button>
</div>
)}
resetKeys={[refreshKey]}
>
<RSCRoute componentName="LiveActivity" componentProps={{ simulateError, refreshKey }} />
</ErrorBoundary>
</div>
);
};

export default LiveActivityRefresher;
58 changes: 58 additions & 0 deletions client/app/bundles/server-components/components/ServerInfo.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Server Component - uses Node.js os module, which only exists on the server.
// This component and its dependencies are never sent to the browser.

import React from 'react';
import os from 'os';
import _ from 'lodash';

function ServerInfo() {
const serverInfo = {
platform: os.platform(),
arch: os.arch(),
nodeVersion: process.version,
uptime: Math.floor(os.uptime() / 3600),
totalMemory: (os.totalmem() / (1024 * 1024 * 1024)).toFixed(1),
freeMemory: (os.freemem() / (1024 * 1024 * 1024)).toFixed(1),
cpus: os.cpus().length,
hostname: os.hostname(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.hostname() is included in the RSC Flight payload and therefore visible in the raw HTTP response body, the browser DevTools network panel, and any CDN cache. Even for a demo, leaking the actual server hostname in production is worth suppressing — it aids host enumeration. Consider either omitting it from the serverInfo object or masking it to a fixed string outside development:

Suggested change
hostname: os.hostname(),
hostname: process.env.NODE_ENV === 'development' ? os.hostname() : '(hidden in production)',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.hostname() reveals an internal server hostname that an attacker can use to fingerprint or target infrastructure. Combined with Node version, free RAM, and CPU count, this is meaningful information disclosure if the page is ever reachable without authentication.

For a demo that ships to a public review app, consider either gating the component behind Rails.env.development? (passed as a prop from the server), omitting hostname, or adding a note in the view that the page must sit behind auth before production use.

};

// Using lodash on the server — this 70KB+ library stays server-side
const infoEntries = _.toPairs(serverInfo);
const grouped = _.chunk(infoEntries, 4);

const labels = {
platform: 'Platform',
arch: 'Architecture',
nodeVersion: 'Node.js',
uptime: 'Uptime (hrs)',
totalMemory: 'Total RAM (GB)',
freeMemory: 'Free RAM (GB)',
cpus: 'CPU Cores',
hostname: 'Hostname',
};

return (
<div className="bg-gradient-to-br from-emerald-50 to-teal-50 border border-emerald-200 rounded-xl p-6">
<p className="text-xs text-emerald-600 mb-4 font-medium">
This data comes from the Node.js <code className="bg-emerald-100 px-1 rounded">os</code> module
— it runs only on the server. The <code className="bg-emerald-100 px-1 rounded">lodash</code> library
used to format it never reaches the browser.
</p>
<div className="grid md:grid-cols-2 gap-x-8 gap-y-1">
{grouped.map((group) => (
<div key={group.map(([k]) => k).join('-')} className="space-y-1">
{group.map(([key, value]) => (
<div key={key} className="flex justify-between py-1.5 border-b border-emerald-100 last:border-0">
<span className="text-sm text-emerald-700 font-medium">{labels[key] || key}</span>
<span className="text-sm text-emerald-900 font-mono">{value}</span>
</div>
))}
</div>
))}
</div>
</div>
);
}

export default ServerInfo;
40 changes: 40 additions & 0 deletions client/app/bundles/server-components/components/TogglePanel.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use client';

import React, { useState } from 'react';
import PropTypes from 'prop-types';

const TogglePanel = ({ title, children }) => {
const [isOpen, setIsOpen] = useState(false);

return (
<div className="border border-slate-200 rounded-lg overflow-hidden">
<button
type="button"
onClick={() => setIsOpen((prev) => !prev)}
className="w-full flex items-center justify-between px-4 py-2.5 bg-slate-50 hover:bg-slate-100 transition-colors text-left"
>
<span className="text-sm font-medium text-slate-700">{title}</span>
<svg
className={`w-4 h-4 text-slate-400 transition-transform ${isOpen ? 'rotate-180' : ''}`}
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
>
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M19 9l-7 7-7-7" />
</svg>
</button>
{isOpen && (
<div className="px-4 py-3 bg-white">
{children}
</div>
)}
</div>
);
};

TogglePanel.propTypes = {
title: PropTypes.string.isRequired,
children: PropTypes.node.isRequired,
};

export default TogglePanel;
Loading
Loading