Skip to content

Commit ae374f7

Browse files
committed
Merge remote-tracking branch 'origin/main' into codex/pr-3142-work
* origin/main: docs: amend CHANGELOG entry for #3229 scope drift fix: validate selected JavaScript package manager fix: preserve causes when wrapping render errors (#3230) # Conflicts: # CHANGELOG.md
2 parents b1cc0bb + b31af7e commit ae374f7

11 files changed

Lines changed: 535 additions & 74 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ
4040

4141
#### Fixed
4242

43+
- **Install generator validates the selected JavaScript package manager**: The install generator now checks the manager selected from `REACT_ON_RAILS_PACKAGE_MANAGER`, the `packageManager` field in `package.json`, or a lockfile on disk — instead of passing when any JavaScript package manager is installed. When the selected command is missing, the error names the selected manager, the source that selected it, and the available alternatives. The generator also warns when `REACT_ON_RAILS_PACKAGE_MANAGER` is set to a value outside the supported set (`npm`, `pnpm`, `yarn`, `bun`). Addresses package manager validation from [Issue 1958](https://github.com/shakacode/react_on_rails/issues/1958). [PR 3229](https://github.com/shakacode/react_on_rails/pull/3229) by [justin808](https://github.com/justin808).
44+
- **Server-render error wrapping preserves original causes**: When server rendering catches a non-`Error` thrown value, React on Rails now wraps it with the original value attached as `cause`, making downstream debugging preserve more context. Fixes [Issue 1746](https://github.com/shakacode/react_on_rails/issues/1746). [PR 3230](https://github.com/shakacode/react_on_rails/pull/3230) by [justin808](https://github.com/justin808).
4345
- **`bin/dev` now cleans copied runtime files before startup**: When you duplicate an app directory to run another local dev stack, `bin/dev` now removes copied stale Overmind sockets and stale `tmp/pids/server.pid` files that point to a Puma process running from another app directory. This prevents false startup failures in copied workspaces while still preserving active local sockets and pid files for the current app. [PR 3142](https://github.com/shakacode/react_on_rails/pull/3142) by [justin808](https://github.com/justin808).
4446
- **[Pro]** **Node renderer now exposes `performance` when `supportModules: true`**: React 19's development build of `React.lazy` calls `performance.now()`, which previously threw `ReferenceError: performance is not defined` inside the node renderer's VM context unless users manually added `performance` via `additionalContext`. `performance` is now included in the default globals alongside `Buffer`, `process`, etc. Fixes [Issue 3154](https://github.com/shakacode/react_on_rails/issues/3154). [PR 3158](https://github.com/shakacode/react_on_rails/pull/3158) by [justin808](https://github.com/justin808).
4547
- **Client startup now recovers if initialization begins during `interactive` after `DOMContentLoaded` already fired**: React on Rails now still initializes the page when the client bundle starts in the browser timing window after `DOMContentLoaded` but before the document reaches `complete`. Fixes [Issue 3150](https://github.com/shakacode/react_on_rails/issues/3150). [PR 3151](https://github.com/shakacode/react_on_rails/pull/3151) by [ihabadham](https://github.com/ihabadham).

packages/react-on-rails-pro-node-renderer/tests/serverRenderRSCReactComponent.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ describe('serverRenderRSCReactComponent', () => {
142142

143143
if (expectedError) {
144144
expect(onError).toHaveBeenCalled();
145-
expect(onError).toHaveBeenCalledWith(new Error(expectedError));
145+
const [emittedError] = onError.mock.calls[0];
146+
expect(Object.prototype.toString.call(emittedError)).toBe('[object Error]');
147+
expect(emittedError.message).toBe(expectedError);
146148
}
147149

148150
expectedContents.forEach((text) => {

packages/react-on-rails/src/serverRenderUtils.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,37 @@ export function createResultObject(
2424
};
2525
}
2626

27+
function isCrossRealmError(e: unknown): e is { message?: unknown } {
28+
return typeof e === 'object' && e !== null && Object.prototype.toString.call(e) === '[object Error]';
29+
}
30+
31+
function stringifyThrownValue(e: unknown): string {
32+
if (isCrossRealmError(e)) {
33+
return typeof e.message === 'string' ? e.message : Object.prototype.toString.call(e);
34+
}
35+
36+
if (typeof e === 'object' && e !== null) {
37+
try {
38+
// JSON.stringify can return undefined without throwing, for example when toJSON returns undefined.
39+
return JSON.stringify(e) ?? Object.prototype.toString.call(e);
40+
} catch {
41+
return Object.prototype.toString.call(e);
42+
}
43+
}
44+
45+
return String(e);
46+
}
47+
2748
export function convertToError(e: unknown): Error {
28-
return e instanceof Error ? e : new Error(String(e));
49+
if (e instanceof Error) {
50+
return e;
51+
}
52+
53+
const message = stringifyThrownValue(e);
54+
// tsconfig uses es2020 libs, which do not type Error.cause even though supported runtimes provide it.
55+
const error = new Error(message) as Error & { cause?: unknown };
56+
error.cause = e;
57+
return error;
2958
}
3059

3160
export function validateComponent(componentObj: RegisteredComponent, componentName: string) {
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { Script } from 'node:vm';
2+
3+
import { convertToError } from '../src/serverRenderUtils.ts';
4+
5+
type ErrorWithCause = Error & { cause?: unknown };
6+
7+
describe('serverRenderUtils', () => {
8+
describe('convertToError', () => {
9+
it('returns Error instances unchanged', () => {
10+
const error = new Error('Already an error');
11+
12+
expect(convertToError(error)).toBe(error);
13+
});
14+
15+
it('wraps plain object thrown values with a readable message while preserving the original cause', () => {
16+
const thrownValue = { message: 'plain object failure' };
17+
18+
const error = convertToError(thrownValue);
19+
20+
expect(error).toBeInstanceOf(Error);
21+
expect(error.message).toBe('{"message":"plain object failure"}');
22+
expect((error as ErrorWithCause).cause).toBe(thrownValue);
23+
});
24+
25+
it('wraps a thrown string with the string as the message and cause', () => {
26+
const error = convertToError('something went wrong');
27+
28+
expect(error).toBeInstanceOf(Error);
29+
expect(error.message).toBe('something went wrong');
30+
expect((error as ErrorWithCause).cause).toBe('something went wrong');
31+
});
32+
33+
it('wraps a thrown number', () => {
34+
const error = convertToError(42);
35+
36+
expect(error).toBeInstanceOf(Error);
37+
expect(error.message).toBe('42');
38+
expect((error as ErrorWithCause).cause).toBe(42);
39+
});
40+
41+
it('wraps null thrown values', () => {
42+
const error = convertToError(null);
43+
44+
expect(error).toBeInstanceOf(Error);
45+
expect(error.message).toBe('null');
46+
expect((error as ErrorWithCause).cause).toBeNull();
47+
});
48+
49+
it('wraps undefined thrown values', () => {
50+
const error = convertToError(undefined);
51+
52+
expect(error).toBeInstanceOf(Error);
53+
expect(error.message).toBe('undefined');
54+
expect((error as ErrorWithCause).cause).toBeUndefined();
55+
});
56+
57+
it('wraps circular-reference objects without throwing', () => {
58+
const circular: Record<string, unknown> = {};
59+
circular.self = circular;
60+
61+
const error = convertToError(circular);
62+
63+
expect(error).toBeInstanceOf(Error);
64+
expect(error.message).toBe('[object Object]');
65+
expect((error as ErrorWithCause).cause).toBe(circular);
66+
});
67+
68+
it('wraps errors thrown from another JavaScript realm with their original message', () => {
69+
const thrownValue: unknown = new Script('new Error("cross-realm failure")').runInNewContext();
70+
71+
const error = convertToError(thrownValue);
72+
73+
expect(error).toBeInstanceOf(Error);
74+
expect(error.message).toBe('cross-realm failure');
75+
expect((error as ErrorWithCause).cause).toBe(thrownValue);
76+
});
77+
78+
it('wraps cross-realm errors with non-string messages using the error tag', () => {
79+
const thrownValue: unknown = new Script(
80+
'const error = new Error(); error.message = 42; error',
81+
).runInNewContext();
82+
83+
const error = convertToError(thrownValue);
84+
85+
expect(error).toBeInstanceOf(Error);
86+
expect(error.message).toBe('[object Error]');
87+
expect((error as ErrorWithCause).cause).toBe(thrownValue);
88+
});
89+
});
90+
});

react_on_rails/lib/generators/react_on_rails/generator_messages/package_manager_detection.rb

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require "json"
4+
require "react_on_rails/utils"
45

56
module GeneratorMessages
67
# Package-manager detection helpers used by the install generator and the
@@ -9,6 +10,15 @@ module GeneratorMessages
910
module PackageManagerDetection
1011
SUPPORTED_PACKAGE_MANAGERS = %w[npm pnpm yarn bun].freeze
1112

13+
# Hash insertion order is the detection priority used by
14+
# detect_package_manager_from_lockfiles (yarn → pnpm → bun → npm).
15+
LOCKFILE_CANDIDATES_BY_MANAGER = {
16+
"yarn" => ["yarn.lock"],
17+
"pnpm" => ["pnpm-lock.yaml"],
18+
"bun" => ["bun.lock", "bun.lockb"],
19+
"npm" => ["package-lock.json"]
20+
}.freeze
21+
1222
# Detects the package manager in priority order:
1323
# 1. REACT_ON_RAILS_PACKAGE_MANAGER env variable
1424
# 2. packageManager field in package.json (Corepack standard)
@@ -20,15 +30,26 @@ module PackageManagerDetection
2030
# Pass package_json: to reuse an already-parsed package.json and avoid a re-read
2131
# (callers that also inspect scripts/deps should parse once and pass the hash).
2232
def detect_package_manager(app_root: Dir.pwd, package_json: nil)
33+
detect_package_manager_with_source(app_root: app_root, package_json: package_json).first
34+
end
35+
36+
# source is one of :env, :package_json, :lockfile, :default — used to
37+
# name the originating source when surfacing detection errors.
38+
def detect_package_manager_with_source(app_root: Dir.pwd, package_json: nil)
2339
env_package_manager = ENV.fetch("REACT_ON_RAILS_PACKAGE_MANAGER", nil)&.strip&.downcase
24-
return env_package_manager if supported_package_manager?(env_package_manager)
40+
return [env_package_manager, :env] if supported_package_manager?(env_package_manager)
2541

2642
pm_from_json = if package_json
2743
package_manager_from_content(package_json)
2844
else
2945
detect_package_manager_from_package_json(app_root: app_root)
3046
end
31-
pm_from_json || detect_package_manager_from_lockfiles(app_root: app_root) || "npm"
47+
return [pm_from_json, :package_json] if pm_from_json
48+
49+
pm_from_lockfile = detect_package_manager_from_lockfiles(app_root: app_root)
50+
return [pm_from_lockfile, :lockfile] if pm_from_lockfile
51+
52+
["npm", :default]
3253
end
3354

3455
def package_manager_from_content(content)
@@ -39,35 +60,35 @@ def package_manager_from_content(content)
3960
supported_package_manager?(name) ? name : nil
4061
end
4162

42-
def detect_package_manager_from_lockfiles(app_root: Dir.pwd)
43-
return "yarn" if File.exist?(File.join(app_root, "yarn.lock"))
44-
return "pnpm" if File.exist?(File.join(app_root, "pnpm-lock.yaml"))
45-
return "bun" if File.exist?(File.join(app_root, "bun.lock")) || File.exist?(File.join(app_root, "bun.lockb"))
46-
return "npm" if File.exist?(File.join(app_root, "package-lock.json"))
47-
48-
nil
63+
def lockfile_filename_for(package_manager, app_root: Dir.pwd)
64+
LOCKFILE_CANDIDATES_BY_MANAGER[package_manager]&.find do |name|
65+
File.exist?(File.join(app_root, name))
66+
end
4967
end
5068

51-
# Returns true only when a lockfile for the specific package manager exists.
52-
# Used by the CI scaffold so `cache:` / `<pm> install` never reference a
53-
# lockfile that is not actually on disk (e.g. `packageManager: pnpm` without
54-
# `pnpm-lock.yaml`, which breaks `actions/setup-node`'s cache step).
69+
# Used by the CI scaffold so `cache:` / `<pm> install` never reference a lockfile
70+
# that's not on disk (e.g. `packageManager: pnpm` without `pnpm-lock.yaml`, which
71+
# breaks `actions/setup-node`'s cache step).
5572
def lockfile_for_manager?(package_manager, app_root: Dir.pwd)
56-
case package_manager
57-
when "yarn" then File.exist?(File.join(app_root, "yarn.lock"))
58-
when "pnpm" then File.exist?(File.join(app_root, "pnpm-lock.yaml"))
59-
when "bun"
60-
File.exist?(File.join(app_root, "bun.lock")) ||
61-
File.exist?(File.join(app_root, "bun.lockb"))
62-
when "npm" then File.exist?(File.join(app_root, "package-lock.json"))
63-
else false
73+
!lockfile_filename_for(package_manager, app_root: app_root).nil?
74+
end
75+
76+
def detect_package_manager_from_lockfiles(app_root: Dir.pwd)
77+
LOCKFILE_CANDIDATES_BY_MANAGER.keys.find do |pm|
78+
lockfile_for_manager?(pm, app_root: app_root)
6479
end
6580
end
6681

6782
def supported_package_manager?(package_manager)
6883
SUPPORTED_PACKAGE_MANAGERS.include?(package_manager)
6984
end
7085

86+
def package_manager_executable_available?(package_manager)
87+
return false unless supported_package_manager?(package_manager)
88+
89+
ReactOnRails::Utils.command_available?(package_manager)
90+
end
91+
7192
private
7293

7394
# Pipeline internals — external callers should go through `detect_package_manager`

react_on_rails/lib/generators/react_on_rails/install_generator.rb

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,8 @@ def ensure_jsx_in_js_compatibility
417417
# js(.coffee) are not checked by this method, but instead produce warning messages
418418
# and allow the build to continue
419419
def installation_prerequisites_met?
420+
warn_if_unsupported_env_package_manager
421+
420422
# Non-blocking: warn about dirty worktree but don't prevent installation.
421423
# A clean tree makes the generator diff easier to review, but blocking would
422424
# be too strict for a generator that creates many new files.
@@ -442,10 +444,21 @@ def installation_prerequisites_met?
442444
!(missing_node? || missing_package_manager? || (!has_worktree_issues && missing_pro_gem?))
443445
end
444446

445-
def missing_node?
446-
node_missing = ReactOnRails::Utils.running_on_windows? ? `where node`.blank? : `which node`.blank?
447+
def warn_if_unsupported_env_package_manager
448+
env_value = ENV.fetch("REACT_ON_RAILS_PACKAGE_MANAGER", nil)&.strip
449+
return if env_value.nil? || env_value.empty?
450+
return if GeneratorMessages.supported_package_manager?(env_value.downcase)
451+
452+
supported = GeneratorMessages::SUPPORTED_PACKAGE_MANAGERS.join(", ")
453+
GeneratorMessages.add_warning(<<~MSG.strip)
454+
⚠️ REACT_ON_RAILS_PACKAGE_MANAGER='#{env_value}' is not a supported package manager.
455+
Supported values: #{supported}.
456+
Falling through to package.json / lockfile / npm-default detection.
457+
MSG
458+
end
447459

448-
if node_missing
460+
def missing_node?
461+
unless ReactOnRails::Utils.command_available?("node")
449462
error = <<~MSG.strip
450463
🚫 Node.js is required but not found on your system.
451464
@@ -734,8 +747,7 @@ def shakapacker_in_gemfile_text?(gem_name)
734747
end
735748

736749
def cli_exists?(command)
737-
which_command = ReactOnRails::Utils.running_on_windows? ? "where" : "which"
738-
system(which_command, command, out: File::NULL, err: File::NULL)
750+
ReactOnRails::Utils.command_available?(command)
739751
end
740752

741753
def normalize_bin_dev_content(content)
@@ -946,13 +958,19 @@ def handle_shakapacker_install_error
946958
end
947959

948960
def missing_package_manager?
949-
package_managers = %w[npm pnpm yarn bun]
950-
missing = package_managers.none? { |pm| cli_exists?(pm) }
961+
selected, source = GeneratorMessages.detect_package_manager_with_source(app_root: destination_root)
962+
return false if GeneratorMessages.package_manager_executable_available?(selected)
951963

952-
if missing
964+
available_package_managers = GeneratorMessages::SUPPORTED_PACKAGE_MANAGERS.select do |pm|
965+
pm != selected && GeneratorMessages.package_manager_executable_available?(pm)
966+
end
967+
968+
if available_package_managers.empty?
953969
error = <<~MSG.strip
954970
🚫 No JavaScript package manager found on your system.
955971
972+
#{package_manager_source_description(selected, source)}
973+
956974
React on Rails requires a JavaScript package manager to install dependencies.
957975
Please install one of the following:
958976
@@ -967,7 +985,32 @@ def missing_package_manager?
967985
return true
968986
end
969987

970-
false
988+
action_separator = %i[default env].include?(source) ? " or " : ", update the source above, or "
989+
error = <<~MSG.strip
990+
🚫 JavaScript package manager '#{selected}' was selected, but the command was not found.
991+
992+
#{package_manager_source_description(selected, source)}
993+
Install '#{selected}'#{action_separator}set REACT_ON_RAILS_PACKAGE_MANAGER
994+
to one of the available package managers: #{available_package_managers.join(', ')}.
995+
MSG
996+
GeneratorMessages.add_error(error)
997+
true
998+
end
999+
1000+
def package_manager_source_description(selected, source)
1001+
case source
1002+
when :env
1003+
"Selected via the REACT_ON_RAILS_PACKAGE_MANAGER environment variable."
1004+
when :package_json
1005+
"Selected via the `packageManager` field in package.json."
1006+
when :lockfile
1007+
lockfile = GeneratorMessages.lockfile_filename_for(selected, app_root: destination_root)
1008+
lockfile ? "Selected via the #{lockfile} lockfile on disk." : "Selected via a lockfile on disk."
1009+
when :default
1010+
"Selected via the npm default fallback (no env var, packageManager field, or lockfile detected)."
1011+
else
1012+
raise ArgumentError, "Unknown package manager source: #{source.inspect}"
1013+
end
9711014
end
9721015

9731016
def jsx_in_js_files_present?

react_on_rails/lib/react_on_rails/utils.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ def self.running_on_windows?
161161
(/cygwin|mswin|mingw|bccwin|wince|emx/ =~ RUBY_PLATFORM) != nil
162162
end
163163

164+
def self.command_available?(command)
165+
which_command = running_on_windows? ? "where" : "which"
166+
!!system(which_command, command, out: File::NULL, err: File::NULL)
167+
end
168+
164169
def self.rails_version_less_than(version)
165170
@rails_version_less_than ||= {}
166171

0 commit comments

Comments
 (0)