Skip to content

Move React/Shakapacker version compatibility to generator smoke tests #2125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
justin808 wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Move React/Shakapacker version compatibility to generator smoke tests #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
504f499
Move React/Shakapacker version compatibility to generator smoke tests
justin808 Nov 25, 2025
8869a2c
Add JSON parse error handling in apply_minimum_versions
justin808 Nov 25, 2025
6f5b0e5
Add minimum_versions? predicate method for Ruby idiom
justin808 Nov 25, 2025
be2c39e
Update spec/dummy for React 19 compatibility
justin808 Nov 25, 2025
175322f
Fix client-side HelmetProvider requirement for @dr.pogodin/react-helmet
justin808 Nov 25, 2025
4eb4575
Fix webpack-assets-manifest version for minimum Shakapacker compatibi…
justin808 Nov 25, 2025
816ae7d
Fix Shakapacker gem/npm version mismatch for minimum version testing
justin808 Nov 25, 2025
7bc7ff7
Fix bundler isolation for generated example apps
justin808 Nov 25, 2025
d62b9aa
Only use BUNDLE_GEMFILE isolation for minimum version examples
justin808 Nov 25, 2025
d881bf1
Update pnpm lockfile for React 19.0.0
justin808 Nov 26, 2025
b255da8
Trigger CI
justin808 Nov 26, 2025
7582e68
Fix bundler isolation for generate_packs in example apps
justin808 Nov 26, 2025
f666831
Run RSpec for minimum version examples with bundler isolation
justin808 Nov 26, 2025
3ca2f9f
Remove deprecated chromedriver-helper from dev_tests generator
justin808 Nov 26, 2025
12fc9d2
Address PR review comments
justin808 Dec 5, 2025
f36c4ff
Address PR review nitpicks and style suggestions
justin808 Dec 7, 2025
a07f022
Fix TypeScript compatibility with @types/react-dom@19
justin808 Dec 7, 2025
4f868c5
Fix ConnectionPool.new to use keyword arguments for Ruby 3.4
justin808 Dec 7, 2025
7288eef
Update pnpm lockfile after rebase
justin808 Dec 8, 2025
a68cd63
Fix minimum version examples by adding npm overrides for React 18
justin808 Dec 9, 2025
33d6fc3
Add React 17 and 18 compatibility testing to generator examples
justin808 Dec 9, 2025
5532c5d
Regenerate Shakapacker binstubs after version downgrade
justin808 Dec 9, 2025
f353f28
Fix workflow step name to document minimum supported versions
justin808 Dec 9, 2025
a96560a
Fix Shakapacker version mismatch by running npm install before binstubs
justin808 Dec 9, 2025
d48f229
Add runtime validation for legacy React APIs
justin808 Dec 9, 2025
2249352
Fix YAML syntax error in examples workflow step name
justin808 Dec 9, 2025
a28f819
Remove outdated commented code from script/convert
justin808 Dec 9, 2025
7c10881
Output webpack build errors to stderr for CI visibility
justin808 Dec 9, 2025
aac91b8
Add babel-loader dependency for Shakapacker 8.2.0 compatibility
justin808 Dec 9, 2025
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
10 changes: 7 additions & 3 deletions .github/workflows/examples.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ jobs:
echo "Node version: "; node -v
echo "pnpm version: "; pnpm --version
echo "Bundler version: "; bundle --version
- name: run conversion script to support shakapacker v6
- name: Run conversion script for older Node compatibility
if: matrix.dependency-level == 'minimum'
run: script/convert
- name: Save root ruby gems to cache
Expand Down Expand Up @@ -180,8 +180,12 @@ jobs:
- name: Set packer version environment variable
run: |
echo "CI_DEPENDENCY_LEVEL=${{ matrix.dependency-level }}" >> $GITHUB_ENV
- name: Main CI
run: cd react_on_rails && bundle exec rake run_rspec:shakapacker_examples
- name: Main CI - Latest version examples
if: matrix.dependency-level == 'latest'
run: cd react_on_rails && bundle exec rake run_rspec:shakapacker_examples_latest
- name: "Main CI - Pinned version examples (minimum: React 18.0.0, Shakapacker 8.2.0)"
if: matrix.dependency-level == 'minimum'
run: cd react_on_rails && bundle exec rake run_rspec:shakapacker_examples_pinned
Comment on lines +183 to +188
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find react_on_rails -name "run_rspec.rake" -o -name "*rspec*.rake" | head -20

Repository: shakacode/react_on_rails

Length of output: 105

🏁 Script executed:

rg -A2 'desc.*shakapacker_examples' --type ruby

Repository: shakacode/react_on_rails

Length of output: 50

🏁 Script executed:

rg -n 'task.*:shakapacker_examples' --type ruby

Repository: shakacode/react_on_rails

Length of output: 50

🏁 Script executed:

cat -n react_on_rails/rakelib/run_rspec.rake | head -150

Repository: shakacode/react_on_rails

Length of output: 7221

🏁 Script executed:

rg -i 'shakapacker_examples' react_on_rails/rakelib/run_rspec.rake -B2 -A2

Repository: shakacode/react_on_rails

Length of output: 2659

Change rake task name on line 188 to match the step's minimum version specification.

Line 188 uses run_rspec:shakapacker_examples_pinned, but the step's description and condition (matrix.dependency-level == 'minimum') indicate it should run only minimum-supported version examples (React 18.0.0, Shakapacker 8.2.0). The correct task is run_rspec:shakapacker_examples_minimum. The shakapacker_examples_pinned task runs all pinned versions (both React 17 and 18), which is inconsistent with the "minimum" condition and label.

🤖 Prompt for AI Agents 
.github/workflows/examples.yml around lines 183 to 188: the "minimum" matrix job
currently runs the rake task run_rspec:shakapacker_examples_pinned which
executes all pinned versions; change the task name to
run_rspec:shakapacker_examples_minimum so the step runs only the
minimum-supported examples (React 18.0.0, Shakapacker 8.2.0) to match the step
label and condition.

- name: Store test results
uses: actions/upload-artifact@v4
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ jobs:
echo "Node version: "; node -v
echo "pnpm version: "; pnpm --version
echo "Bundler version: "; bundle --version
- name: run conversion script to support shakapacker v6
- name: Run conversion script for older Node compatibility
if: matrix.dependency-level == 'minimum'
run: script/convert
- name: Install Node modules with pnpm for renderer package
Expand Down Expand Up @@ -229,7 +229,7 @@ jobs:
echo "Node version: "; node -v
echo "pnpm version: "; pnpm --version
echo "Bundler version: "; bundle --version
- name: run conversion script to support shakapacker v6
- name: Run conversion script for older Node compatibility
if: matrix.dependency-level == 'minimum'
run: script/convert
- name: Save root ruby gems to cache
Expand Down
9 changes: 5 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
"@tsconfig/node14": "^14.1.2",
"@types/jest": "^29.5.14",
"@types/node": "^20.17.16",
"@types/react": "^18.3.18",
"@types/react-dom": "^18.3.5",
"@types/react": "^19.0.0",
"@types/react-dom": "^19.0.0",
"@types/turbolinks": "^5.2.2",
"create-react-class": "^15.7.0",
"eslint": "^9.24.0",
Expand All @@ -58,8 +58,9 @@
"prettier": "^3.5.2",
"prop-types": "^15.8.1",
"publint": "^0.3.4",
"react": "18.0.0",
"react-dom": "18.0.0",
"react": "^19.0.0",
"react-dom": "^19.0.0",
"react-on-rails-rsc": "19.0.2",
Comment on lines +61 to +63
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 8, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if package.json ends with newline
echo "=== Checking package.json file ending ==="
tail -c 5 package.json | od -c

echo ""
echo "=== Full tail of package.json (last 10 lines) ==="
tail -10 package.json

Repository: shakacode/react_on_rails

Length of output: 498

🏁 Script executed:

#!/bin/bash
# Search for potential React 19 compatibility issues

echo "=== Checking for forwardRef usage (deprecated in React 19) ==="
rg -n "forwardRef" --type=jsx --type=tsx --type=js --type=ts -g '!node_modules' -g '!vendor' -C2

echo ""
echo "=== Checking for legacy context usage (removed in React 19) ==="
rg -n "contextTypes|getChildContext" --type=jsx --type=tsx --type=js --type=ts -g '!node_modules' -g '!vendor' -C2

echo ""
echo "=== Checking for string refs (removed in React 19) ==="
rg -n 'ref="' --type=jsx --type=tsx -g '!node_modules' -g '!vendor' -C2

echo ""
echo "=== Checking for ReactDOM.render usage (removed in React 19) ==="
rg -n "ReactDOM\.(render|hydrate|unmountComponentAtNode)" --type=jsx --type=tsx --type=js --type=ts -g '!node_modules' -g '!vendor' -C2

Repository: shakacode/react_on_rails

Length of output: 446

🏁 Script executed:

#!/bin/bash
# Check exact file ending
echo "=== Checking package.json exact file ending ==="
tail -c 10 package.json | xxd

echo ""
echo "=== Last character check ==="
tail -c 1 package.json | wc -l

Repository: shakacode/react_on_rails

Length of output: 206

🏁 Script executed:

#!/bin/bash
# Search for React 19 compatibility issues without restrictive file type filters

echo "=== Checking for forwardRef usage ==="
rg -n "forwardRef" -g '!node_modules' -g '!vendor' -C2

echo ""
echo "=== Checking for legacy context usage ==="
rg -n "contextTypes|getChildContext" -g '!node_modules' -g '!vendor' -C2

echo ""
echo "=== Checking for string refs ==="
rg -n 'ref="' -g '!node_modules' -g '!vendor' -C2 | head -50

echo ""
echo "=== Checking for ReactDOM.render usage ==="
rg -n "ReactDOM\.(render|hydrate|unmountComponentAtNode)" -g '!node_modules' -g '!vendor' -C2

Repository: shakacode/react_on_rails

Length of output: 50383

🏁 Script executed:

#!/bin/bash
# Check for forwardRef in source code (excluding test fixtures and bundles)
echo "=== Checking for forwardRef in source code only ==="
rg -n "forwardRef" -g '!node_modules' -g '!vendor' -g '!tests/fixtures' -g '!*.bundle.js' -g '!**/dist/**' -C1

echo ""
echo "=== Checking app/components directory specifically ==="
fd -e js -e jsx -e ts -e tsx app/ src/ 2>/dev/null | head -20

echo ""
echo "=== ReactDOM deprecated methods in source ==="
rg -n "ReactDOM\.(render|hydrate|unmountComponentAtNode)" -g '!node_modules' -g '!vendor' -g '!tests/fixtures' -C2

Repository: shakacode/react_on_rails

Length of output: 50382

Ensure package.json ends with a newline character.

The file currently does not end with a newline, violating the coding guidelines that require all files to end with a newline before committing.

🤖 Prompt for AI Agents 
In package.json around lines 61 to 63 the file currently does not end with a
newline; add a single newline character at the end of the file (ensure the final
byte is a newline) and save so the file terminates with a newline, then re-run
linters/commit.

"redux": "^4.2.1",
"size-limit": "^12.0.0",
"stylelint": "^16.14.0",
Expand Down
1 change: 0 additions & 1 deletion packages/react-on-rails/src/ClientRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ function unmountAllComponents(): void {
root.unmount();
} else {
// React 16-17 legacy API
// eslint-disable-next-line @typescript-eslint/no-deprecated
unmountComponentAtNode(domNode);
}
} catch (error) {
Expand Down
41 changes: 32 additions & 9 deletions packages/react-on-rails/src/reactApis.cts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import * as ReactDOM from 'react-dom';
import type { ReactElement } from 'react';
import type { RenderReturnType } from './types/index.ts' with { 'resolution-mode': 'import' };

// Type for legacy React DOM APIs (React 16/17) that were removed from @types/react-dom@19
// These are only used at runtime when supportsRootApi is false
interface LegacyReactDOM {
hydrate(element: ReactElement, container: Element): void;
render(element: ReactElement, container: Element): RenderReturnType;
unmountComponentAtNode(container: Element): boolean;
}

const reactMajorVersion = Number(ReactDOM.version?.split('.')[0]) || 16;

// TODO: once we require React 18, we can remove this and inline everything guarded by it.
Expand All @@ -29,12 +37,27 @@ if (supportsRootApi) {

type HydrateOrRenderType = (domNode: Element, reactElement: ReactElement) => RenderReturnType;

/* eslint-disable @typescript-eslint/no-deprecated,@typescript-eslint/no-non-null-assertion,react/no-deprecated --
* while we need to support React 16
*/
// Cast ReactDOM to include legacy APIs for React 16/17 compatibility
// These methods exist at runtime but are removed from @types/react-dom@19
const legacyReactDOM = ReactDOM as unknown as LegacyReactDOM;

// Validate legacy APIs exist at runtime when needed (React < 18)
if (!supportsRootApi) {
if (typeof legacyReactDOM.hydrate !== 'function') {
throw new Error('React legacy hydrate API not available. Expected React 16/17.');
}
if (typeof legacyReactDOM.render !== 'function') {
throw new Error('React legacy render API not available. Expected React 16/17.');
}
if (typeof legacyReactDOM.unmountComponentAtNode !== 'function') {
throw new Error('React legacy unmountComponentAtNode API not available. Expected React 16/17.');
}
}

/* eslint-disable @typescript-eslint/no-non-null-assertion -- reactDomClient is always defined when supportsRootApi is true */
export const reactHydrate: HydrateOrRenderType = supportsRootApi
? reactDomClient!.hydrateRoot
: (domNode, reactElement) => ReactDOM.hydrate(reactElement, domNode);
: (domNode, reactElement) => legacyReactDOM.hydrate(reactElement, domNode);

export function reactRender(domNode: Element, reactElement: ReactElement): RenderReturnType {
if (supportsRootApi) {
Expand All @@ -43,14 +66,14 @@ export function reactRender(domNode: Element, reactElement: ReactElement): Rende
return root;
}

// eslint-disable-next-line react/no-render-return-value
return ReactDOM.render(reactElement, domNode);
return legacyReactDOM.render(reactElement, domNode);
}

export const unmountComponentAtNode: typeof ReactDOM.unmountComponentAtNode = supportsRootApi
export const unmountComponentAtNode: (container: Element) => boolean = supportsRootApi
? // not used if we use root API
() => false
: ReactDOM.unmountComponentAtNode;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
(_container: Element) => false
: (container: Element) => legacyReactDOM.unmountComponentAtNode(container);

export const ensureReactUseAvailable = () => {
if (!('use' in React) || typeof React.use !== 'function') {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-on-rails/src/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ interface ServerRenderResult {
error?: Error;
}

type CreateReactOutputSyncResult = ServerRenderResult | ReactElement<unknown>;
type CreateReactOutputSyncResult = ServerRenderResult | ReactElement;

type CreateReactOutputAsyncResult = Promise<string | ServerRenderHashRenderedHtml | ReactElement<unknown>>;
type CreateReactOutputAsyncResult = Promise<string | ServerRenderHashRenderedHtml | ReactElement>;

type CreateReactOutputResult = CreateReactOutputSyncResult | CreateReactOutputAsyncResult;

Expand Down
Loading
Loading