|
| 1 | +# Issue: Add Playwright Test to Catch Turbo Navigation Regression |
| 2 | + |
| 3 | +## Summary |
| 4 | + |
| 5 | +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. |
| 6 | + |
| 7 | +## Background |
| 8 | + |
| 9 | +### What Happened |
| 10 | + |
| 11 | +**Bug introduced in:** PR #1620 (commit 56ee2bd9, ~1 year ago) |
| 12 | + |
| 13 | +- Added Turbo (Hotwire) support to replace Turbolinks |
| 14 | +- Updated client-bundle.js to import `@hotwired/turbo-rails` |
| 15 | +- Set `turbo: true` in ReactOnRails options |
| 16 | +- **Forgot to update layout file** from `data-turbolinks-track` to `data-turbo-track` |
| 17 | + |
| 18 | +**Bug fixed in:** PR #XXXX (commit f03b935d) |
| 19 | + |
| 20 | +- Updated `spec/dummy/app/views/layouts/application.html.erb` |
| 21 | +- Changed `data-turbolinks-track: true` to `data-turbo-track: 'reload'` |
| 22 | +- Re-added `defer: true` for proper Turbo compatibility |
| 23 | + |
| 24 | +### Impact |
| 25 | + |
| 26 | +Users experienced broken JavaScript when: |
| 27 | + |
| 28 | +1. Landing on a page (hard refresh) → ✅ JavaScript works |
| 29 | +2. Clicking a link to navigate → ❌ JavaScript breaks |
| 30 | +3. Hard refreshing again → ✅ JavaScript works again |
| 31 | + |
| 32 | +This severely degraded the user experience with Turbo navigation. |
| 33 | + |
| 34 | +## The Test Gap |
| 35 | + |
| 36 | +This bug went undetected for over a year, indicating a gap in test coverage for: |
| 37 | + |
| 38 | +- Turbo navigation flows |
| 39 | +- JavaScript execution after client-side navigation |
| 40 | +- React component hydration after Turbo page loads |
| 41 | + |
| 42 | +## Proposed Solution: Playwright E2E Test |
| 43 | + |
| 44 | +Add a Playwright test that verifies: |
| 45 | + |
| 46 | +### Test Scenario |
| 47 | + |
| 48 | +```javascript |
| 49 | +test('React components work after Turbo navigation', async ({ page }) => { |
| 50 | + // 1. Hard refresh - load first page |
| 51 | + await page.goto('/react_router'); |
| 52 | + |
| 53 | + // 2. Verify JavaScript works on initial load |
| 54 | + await expect(page.locator('input#name')).toBeVisible(); |
| 55 | + await page.fill('input#name', 'Initial Page'); |
| 56 | + await expect(page.locator('h3')).toContainText('Initial Page'); |
| 57 | + |
| 58 | + // 3. Click a Turbo link to navigate to second page |
| 59 | + await page.click('a[href="/react_router/second_page"]'); |
| 60 | + await page.waitForURL('/react_router/second_page'); |
| 61 | + |
| 62 | + // 4. Verify JavaScript STILL works after Turbo navigation |
| 63 | + await expect(page.locator('input#name')).toBeVisible(); |
| 64 | + await page.fill('input#name', 'After Turbo Navigation'); |
| 65 | + await expect(page.locator('h3')).toContainText('After Turbo Navigation'); |
| 66 | + |
| 67 | + // 5. Navigate back |
| 68 | + await page.click('a[href="/react_router"]'); |
| 69 | + await page.waitForURL('/react_router'); |
| 70 | + |
| 71 | + // 6. Verify JavaScript works after navigating back |
| 72 | + await expect(page.locator('input#name')).toBeVisible(); |
| 73 | + await page.fill('input#name', 'After Back Navigation'); |
| 74 | + await expect(page.locator('h3')).toContainText('After Back Navigation'); |
| 75 | +}); |
| 76 | +``` |
| 77 | + |
| 78 | +### What This Test Catches |
| 79 | + |
| 80 | +✅ JavaScript execution after Turbo navigation |
| 81 | +✅ React component hydration on client-side page loads |
| 82 | +✅ Proper data attributes for Turbo compatibility |
| 83 | +✅ Component lifecycle management with Turbo |
| 84 | + |
| 85 | +## Verification Steps |
| 86 | + |
| 87 | +To verify the Playwright test works correctly: |
| 88 | + |
| 89 | +### Step 1: Ensure Test Passes with Fix |
| 90 | + |
| 91 | +```bash |
| 92 | +# Current state (with fix) - test should PASS |
| 93 | +npm run test:e2e -- --grep "Turbo navigation" |
| 94 | +``` |
| 95 | + |
| 96 | +### Step 2: Revert the Fix |
| 97 | + |
| 98 | +```bash |
| 99 | +# Revert to broken state |
| 100 | +git show f03b935d:spec/dummy/app/views/layouts/application.html.erb > spec/dummy/app/views/layouts/application.html.erb |
| 101 | + |
| 102 | +# Change line 13 back to: |
| 103 | +# <%= javascript_pack_tag('client-bundle', 'data-turbolinks-track': true) %> |
| 104 | +``` |
| 105 | + |
| 106 | +### Step 3: Verify Test Fails |
| 107 | + |
| 108 | +```bash |
| 109 | +# With reverted fix - test should FAIL |
| 110 | +npm run test:e2e -- --grep "Turbo navigation" |
| 111 | + |
| 112 | +# Expected failure: |
| 113 | +# Error: Timeout waiting for element 'input#name' after Turbo navigation |
| 114 | +``` |
| 115 | + |
| 116 | +### Step 4: Restore the Fix |
| 117 | + |
| 118 | +```bash |
| 119 | +# Restore the fix |
| 120 | +git checkout HEAD spec/dummy/app/views/layouts/application.html.erb |
| 121 | + |
| 122 | +# Test should PASS again |
| 123 | +npm run test:e2e -- --grep "Turbo navigation" |
| 124 | +``` |
| 125 | + |
| 126 | +## Implementation Checklist |
| 127 | + |
| 128 | +- [ ] Create Playwright test file: `spec/dummy/spec/playwright/turbo_navigation.spec.js` |
| 129 | +- [ ] Add test that verifies React components work after Turbo navigation |
| 130 | +- [ ] Verify test passes with current fix in place |
| 131 | +- [ ] Manually revert fix and confirm test fails (proves test is effective) |
| 132 | +- [ ] Restore fix and confirm test passes again |
| 133 | +- [ ] Add test to CI pipeline |
| 134 | +- [ ] Document in CONTRIBUTING.md that Turbo navigation must be tested |
| 135 | + |
| 136 | +## Additional Test Cases to Consider |
| 137 | + |
| 138 | +1. **Multiple navigations**: Navigate through 3-4 pages to ensure no memory leaks |
| 139 | +2. **Back/forward buttons**: Test browser back/forward with Turbo |
| 140 | +3. **Turbo Frames**: Test components inside turbo frames |
| 141 | +4. **Turbo Streams**: Test components updated via turbo streams (Pro feature) |
| 142 | +5. **Redux stores**: Verify store state persists/resets appropriately |
| 143 | + |
| 144 | +## Files to Create/Modify |
| 145 | + |
| 146 | +``` |
| 147 | +spec/dummy/spec/playwright/ |
| 148 | + ├── turbo_navigation.spec.js (new) |
| 149 | + └── support/ |
| 150 | + └── turbo_helpers.js (new - optional helper functions) |
| 151 | +
|
| 152 | +.github/workflows/ |
| 153 | + └── playwright.yml (update to run new tests) |
| 154 | +
|
| 155 | +docs/contributor-info/ |
| 156 | + └── testing-turbo-navigation.md (new - documentation) |
| 157 | +``` |
| 158 | + |
| 159 | +## Success Criteria |
| 160 | + |
| 161 | +- [ ] Playwright test exists and passes in CI |
| 162 | +- [ ] Test catches the regression when fix is reverted |
| 163 | +- [ ] Test is documented and maintainable |
| 164 | +- [ ] Test runs in <30 seconds |
| 165 | +- [ ] No false positives in CI |
| 166 | + |
| 167 | +## References |
| 168 | + |
| 169 | +- **Original Turbo PR**: #1620 (56ee2bd9) |
| 170 | +- **Fix commit**: f03b935d |
| 171 | +- **Turbo documentation**: docs/building-features/turbolinks.md |
| 172 | +- **Related issue**: This regression went undetected for 1+ year |
| 173 | + |
| 174 | +## Notes for Implementer |
| 175 | + |
| 176 | +- Use React on Rails Pro dummy app for testing (has more Turbo features) |
| 177 | +- Consider using Playwright's `page.route()` to simulate slow networks |
| 178 | +- Add `data-testid` attributes to components if needed for reliable selectors |
| 179 | +- Test both development and production webpack builds |
| 180 | +- Ensure test works with different Turbo Drive settings (`turbo: true/false`) |
| 181 | + |
| 182 | +--- |
| 183 | + |
| 184 | +**Priority**: High - This prevents regressions in core Turbo functionality |
| 185 | + |
| 186 | +**Estimated effort**: 2-4 hours (including documentation and verification) |
| 187 | + |
| 188 | +**Labels**: testing, playwright, turbo, e2e, regression-prevention |
0 commit comments