Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
8c3bbd6
Update shakapacker dependency to version 9.3.0
justin808 Nov 2, 2025
0a27537
Revert async loading version check to 8.2.0
justin808 Nov 2, 2025
8832a8e
Add swc-loader support for Shakapacker 9.3.0
justin808 Nov 2, 2025
27d7bf1
Fix CSS Modules compatibility with Shakapacker 9.0.0
justin808 Nov 2, 2025
aea9af1
Add swc-loader to React on Rails generator dependencies
justin808 Nov 2, 2025
42c109c
Skip version validation when package.json doesn't exist
justin808 Nov 2, 2025
e15e242
Fix webpack config error with undefined scss rule
justin808 Nov 2, 2025
d96c24a
Fix generator test failures by skipping validation and using exact ve…
justin808 Nov 2, 2025
f44b902
Add draft comment for package_json gem issue #25
justin808 Nov 2, 2025
b5401ef
Fix SCSS undefined variable error by importing app-variables
justin808 Nov 2, 2025
692325d
Fix generator robustness issues: package manager detection, validatio…
justin808 Nov 3, 2025
2a4f03a
Skip version validation when react-on-rails package not installed
justin808 Nov 3, 2025
74c46fd
Fix babel preset import path for Shakapacker 9.3.0
justin808 Nov 3, 2025
7dc4e59
Remove redundant SCSS @import statements
justin808 Nov 3, 2025
418dd9b
Fix ESLint import/extensions error in babel.config.js
justin808 Nov 3, 2025
53dfebe
Fix CodeRabbit review issues: invalid config, version consistency, mi…
justin808 Nov 3, 2025
e867be2
Fix SCSS undefined variable error by importing app-variables
justin808 Nov 3, 2025
60e651c
Move swc-loader and @swc/core to devDependencies in Pro package.json …
justin808 Nov 3, 2025
115d557
Move shakapacker to devDependencies in Pro dummy package.json
justin808 Nov 3, 2025
9d69b27
Fix babel preset require path for Shakapacker 9.3.0 with explicit .js…
justin808 Nov 3, 2025
642a7f7
Fix prepack script to use yarn instead of npm
justin808 Nov 3, 2025
a6036c3
Address code review comments
justin808 Nov 3, 2025
edbb01f
Fix bin/dev pack generation failure when run from Bundler context
justin808 Nov 3, 2025
c0d67ef
Add ReScript build step before starting development server
justin808 Nov 3, 2025
2b76fb8
Fix CI frozen-lockfile error with yalc workflow
justin808 Nov 3, 2025
c26d7ac
Fix Turbo navigation by updating to Turbo-compatible script tag
justin808 Nov 3, 2025
32b951c
Add issue documentation for Playwright test to catch Turbo navigation…
justin808 Nov 3, 2025
dfb7a89
Fix yalc publish command in Pro integration workflow
justin808 Nov 3, 2025
2f7c479
Fix prepack script to use yarn instead of npm in package-scripts.yml
justin808 Nov 3, 2025
bd6bc39
Add language specifier to code fence in TURBO_NAVIGATION_TEST_ISSUE.md
justin808 Nov 3, 2025
6abe076
Update PR placeholder with actual PR number in TURBO_NAVIGATION_TEST_…
justin808 Nov 3, 2025
9e1808a
Remove --frozen-lockfile from dummy app install after yalc
justin808 Nov 3, 2025
d658ab6
Update shakapacker gem version to 9.3.0 in Pro Gemfiles
justin808 Nov 3, 2025
0e79347
Update Pro dummy app yarn.lock for Shakapacker 9.3.0
justin808 Nov 3, 2025
6ad3e5d
Update execjs-compatible-dummy yarn.lock for Shakapacker 9.3.0
justin808 Nov 3, 2025
519a05b
Fix Pro dummy webpack config scss undefined error
justin808 Nov 3, 2025
109b463
Update using_swc? to return true by default for Shakapacker 9.3.0
justin808 Nov 4, 2025
cc2f917
Fix using_swc? to properly parse YAML and default to babel
justin808 Nov 4, 2025
794df98
Address code review feedback
justin808 Nov 4, 2025
1ce2ffe
Remove Shakapacker config changes from generator templates
justin808 Nov 4, 2025
7f902cf
Fix unsafe system calls to use array form with cross-platform output …
justin808 Nov 4, 2025
0993b48
Fix precompile hook to properly locate Rails root
justin808 Nov 4, 2025
b716f08
Remove dead code from run_rake_task_directly method
justin808 Nov 4, 2025
53dea52
Add 'use client' directive to RSCProvider
justin808 Nov 4, 2025
cb444c4
Revert pack_generator system call changes to fix test failures
justin808 Nov 4, 2025
2f26987
Fix React 19 server bundle errors by removing 'use client' from .serv…
justin808 Nov 4, 2025
bf8c2bb
Fix React 19 server bundle errors by removing 'use client' from RSCPr…
justin808 Nov 5, 2025
2402fe8
Add ESLint rule to prevent 'use client' in server files
justin808 Nov 5, 2025
59be8ab
Fix Shakapacker 9.3.0 compatibility issues
justin808 Nov 5, 2025
c68eabe
Remove scratch test files that were accidentally committed
justin808 Nov 5, 2025
68de461
Add Shakapacker precompile hook support for auto pack generation
justin808 Oct 24, 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
2 changes: 1 addition & 1 deletion .github/workflows/lint-js-and-ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ jobs:
run: cd spec/dummy && RAILS_ENV="test" bundle exec rake react_on_rails:generate_packs
- name: Detect dead code
run: |
yarn run knip
yarn run knip --no-config-hints
yarn run knip --production
- name: Lint JS
run: yarn run eslint --report-unused-disable-directives
Expand Down
27 changes: 24 additions & 3 deletions .github/workflows/pro-integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,15 @@ jobs:
sudo yarn global add yalc
yarn install --frozen-lockfile --no-progress --no-emoji

- name: Build and publish packages with yalc
run: |
cd ..
yarn run yalc:publish
cd react_on_rails_pro
yarn && yalc publish

- name: Install Node modules with Yarn for Pro dummy app
run: cd spec/dummy && yarn install --frozen-lockfile --no-progress --no-emoji
run: cd spec/dummy && yarn install --ignore-scripts --no-progress --no-emoji

- name: Install Ruby Gems for Pro dummy app
run: |
Expand Down Expand Up @@ -180,8 +187,15 @@ jobs:
sudo yarn global add yalc
yarn install --frozen-lockfile --no-progress --no-emoji

- name: Build and publish packages with yalc
run: |
cd ..
yarn run yalc:publish
cd react_on_rails_pro
yarn && yalc publish

- name: Install Node modules with Yarn for Pro dummy app
run: cd spec/dummy && yarn install --frozen-lockfile --no-progress --no-emoji
run: cd spec/dummy && yarn install --ignore-scripts --no-progress --no-emoji

- name: Ensure minimum required Chrome version
run: |
Expand Down Expand Up @@ -365,8 +379,15 @@ jobs:
sudo yarn global add yalc
yarn install --frozen-lockfile --no-progress --no-emoji

- name: Build and publish packages with yalc
run: |
cd ..
yarn run yalc:publish
cd react_on_rails_pro
yarn && yalc publish

- name: Install Node modules with Yarn for Pro dummy app
run: cd spec/dummy && yarn install --frozen-lockfile --no-progress --no-emoji
run: cd spec/dummy && yarn install --ignore-scripts --no-progress --no-emoji

- name: Ensure minimum required Chrome version
run: |
Expand Down
4 changes: 2 additions & 2 deletions Gemfile.development_dependencies
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

gem "shakapacker", "8.2.0"
gem "shakapacker", "9.3.0"
gem "bootsnap", require: false
gem "rails", "~> 7.1"

Expand Down Expand Up @@ -46,7 +46,7 @@ group :development, :test do
end

group :test do
gem "capybara"
gem "capybara", ">= 3.40"
gem "capybara-screenshot"
gem "coveralls", require: false
gem "equivalent-xml"
Expand Down
26 changes: 13 additions & 13 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ GEM
minitest (>= 5.1)
mutex_m
tzinfo (~> 2.0)
addressable (2.8.6)
public_suffix (>= 2.0.2, < 6.0)
addressable (2.8.7)
public_suffix (>= 2.0.2, < 7.0)
amazing_print (1.6.0)
ast (2.4.2)
base64 (0.2.0)
Expand Down Expand Up @@ -171,10 +171,10 @@ GEM
net-pop
net-smtp
marcel (1.0.4)
matrix (0.4.2)
matrix (0.4.3)
method_source (1.1.0)
mini_mime (1.1.5)
mini_portile2 (2.8.7)
mini_portile2 (2.8.9)
minitest (5.24.1)
msgpack (1.7.2)
mutex_m (0.2.0)
Expand All @@ -188,7 +188,7 @@ GEM
net-smtp (0.5.0)
net-protocol
nio4r (2.7.3)
nokogiri (1.16.6)
nokogiri (1.18.10)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
ostruct (0.6.1)
Expand All @@ -213,16 +213,16 @@ GEM
pry (>= 0.12.0)
psych (5.1.2)
stringio
public_suffix (5.0.5)
public_suffix (6.0.2)
puma (6.4.2)
nio4r (~> 2.0)
racc (1.8.0)
rack (3.1.4)
racc (1.8.1)
rack (3.2.4)
rack-proxy (0.7.7)
rack
rack-session (2.0.0)
rack (>= 3.0.0)
rack-test (2.1.0)
rack-test (2.2.0)
rack (>= 1.3)
rackup (2.1.0)
rack (>= 3)
Expand Down Expand Up @@ -263,7 +263,7 @@ GEM
ffi (~> 1.0)
rdoc (6.7.0)
psych (>= 4.0.0)
regexp_parser (2.9.2)
regexp_parser (2.11.3)
reline (0.5.9)
io-console (~> 0.5)
rexml (3.2.7)
Expand Down Expand Up @@ -342,7 +342,7 @@ GEM
rubyzip (>= 1.2.2, < 3.0)
websocket (~> 1.0)
semantic_range (3.1.0)
shakapacker (8.2.0)
shakapacker (9.3.0)
activesupport (>= 5.2)
package_json
rack-proxy (>= 0.6.1)
Expand Down Expand Up @@ -407,7 +407,7 @@ DEPENDENCIES
amazing_print
benchmark
bootsnap
capybara
capybara (>= 3.40)
capybara-screenshot
coveralls
debug
Expand Down Expand Up @@ -440,7 +440,7 @@ DEPENDENCIES
scss_lint
sdoc
selenium-webdriver (= 4.9.0)
shakapacker (= 8.2.0)
shakapacker (= 9.3.0)
spring (~> 4.0)
sprockets (~> 4.0)
sqlite3 (~> 1.6)
Expand Down
188 changes: 188 additions & 0 deletions TURBO_NAVIGATION_TEST_ISSUE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
# Issue: Add Playwright Test to Catch Turbo Navigation Regression

## Summary

A regression was discovered where JavaScript fails to work after navigating between pages using Turbo links (only works on hard refresh). This issue existed in the codebase for over a year but wasn't caught by automated tests.

## Background

### What Happened

**Bug introduced in:** PR #1620 (commit 56ee2bd9, ~1 year ago)

- Added Turbo (Hotwire) support to replace Turbolinks
- Updated client-bundle.js to import `@hotwired/turbo-rails`
- Set `turbo: true` in ReactOnRails options
- **Forgot to update layout file** from `data-turbolinks-track` to `data-turbo-track`

**Bug fixed in:** PR #1896 (commit f03b935d)

- Updated `spec/dummy/app/views/layouts/application.html.erb`
- Changed `data-turbolinks-track: true` to `data-turbo-track: 'reload'`
- Re-added `defer: true` for proper Turbo compatibility

### Impact

Users experienced broken JavaScript when:

1. Landing on a page (hard refresh) β†’ βœ… JavaScript works
2. Clicking a link to navigate β†’ ❌ JavaScript breaks
3. Hard refreshing again β†’ βœ… JavaScript works again

This severely degraded the user experience with Turbo navigation.

## The Test Gap

This bug went undetected for over a year, indicating a gap in test coverage for:

- Turbo navigation flows
- JavaScript execution after client-side navigation
- React component hydration after Turbo page loads

## Proposed Solution: Playwright E2E Test

Add a Playwright test that verifies:

### Test Scenario

```javascript
test('React components work after Turbo navigation', async ({ page }) => {
// 1. Hard refresh - load first page
await page.goto('/react_router');

// 2. Verify JavaScript works on initial load
await expect(page.locator('input#name')).toBeVisible();
await page.fill('input#name', 'Initial Page');
await expect(page.locator('h3')).toContainText('Initial Page');

// 3. Click a Turbo link to navigate to second page
await page.click('a[href="/react_router/second_page"]');
await page.waitForURL('/react_router/second_page');

// 4. Verify JavaScript STILL works after Turbo navigation
await expect(page.locator('input#name')).toBeVisible();
await page.fill('input#name', 'After Turbo Navigation');
await expect(page.locator('h3')).toContainText('After Turbo Navigation');

// 5. Navigate back
await page.click('a[href="/react_router"]');
await page.waitForURL('/react_router');

// 6. Verify JavaScript works after navigating back
await expect(page.locator('input#name')).toBeVisible();
await page.fill('input#name', 'After Back Navigation');
await expect(page.locator('h3')).toContainText('After Back Navigation');
});
```

### What This Test Catches

βœ… JavaScript execution after Turbo navigation
βœ… React component hydration on client-side page loads
βœ… Proper data attributes for Turbo compatibility
βœ… Component lifecycle management with Turbo

## Verification Steps

To verify the Playwright test works correctly:

### Step 1: Ensure Test Passes with Fix

```bash
# Current state (with fix) - test should PASS
npm run test:e2e -- --grep "Turbo navigation"
```

### Step 2: Revert the Fix

```bash
# Revert to broken state
git show f03b935d:spec/dummy/app/views/layouts/application.html.erb > spec/dummy/app/views/layouts/application.html.erb

# Change line 13 back to:
# <%= javascript_pack_tag('client-bundle', 'data-turbolinks-track': true) %>
```

### Step 3: Verify Test Fails

```bash
# With reverted fix - test should FAIL
npm run test:e2e -- --grep "Turbo navigation"

# Expected failure:
# Error: Timeout waiting for element 'input#name' after Turbo navigation
```

### Step 4: Restore the Fix

```bash
# Restore the fix
git checkout HEAD spec/dummy/app/views/layouts/application.html.erb

# Test should PASS again
npm run test:e2e -- --grep "Turbo navigation"
```

## Implementation Checklist

- [ ] Create Playwright test file: `spec/dummy/spec/playwright/turbo_navigation.spec.js`
- [ ] Add test that verifies React components work after Turbo navigation
- [ ] Verify test passes with current fix in place
- [ ] Manually revert fix and confirm test fails (proves test is effective)
- [ ] Restore fix and confirm test passes again
- [ ] Add test to CI pipeline
- [ ] Document in CONTRIBUTING.md that Turbo navigation must be tested

## Additional Test Cases to Consider

1. **Multiple navigations**: Navigate through 3-4 pages to ensure no memory leaks
2. **Back/forward buttons**: Test browser back/forward with Turbo
3. **Turbo Frames**: Test components inside turbo frames
4. **Turbo Streams**: Test components updated via turbo streams (Pro feature)
5. **Redux stores**: Verify store state persists/resets appropriately

## Files to Create/Modify

```tree
spec/dummy/spec/playwright/
β”œβ”€β”€ turbo_navigation.spec.js (new)
└── support/
└── turbo_helpers.js (new - optional helper functions)

.github/workflows/
└── playwright.yml (update to run new tests)

docs/contributor-info/
└── testing-turbo-navigation.md (new - documentation)
```

## Success Criteria

- [ ] Playwright test exists and passes in CI
- [ ] Test catches the regression when fix is reverted
- [ ] Test is documented and maintainable
- [ ] Test runs in <30 seconds
- [ ] No false positives in CI

## References

- **Original Turbo PR**: #1620 (56ee2bd9)
- **Fix commit**: f03b935d
- **Turbo documentation**: docs/building-features/turbolinks.md
- **Related issue**: This regression went undetected for 1+ year

## Notes for Implementer

- Use React on Rails Pro dummy app for testing (has more Turbo features)
- Consider using Playwright's `page.route()` to simulate slow networks
- Add `data-testid` attributes to components if needed for reliable selectors
- Test both development and production webpack builds
- Ensure test works with different Turbo Drive settings (`turbo: true/false`)

---

**Priority**: High - This prevents regressions in core Turbo functionality

**Estimated effort**: 2-4 hours (including documentation and verification)

**Labels**: testing, playwright, turbo, e2e, regression-prevention
Loading
Loading