Skip to content

Commit 06f52f5

Browse files
justin808claude
andcommitted
Improve naming clarity in pageLifecycle.ts (#1762)
* Improve naming clarity in pageLifecycle.ts The main improvements: - Rename setupTurbolinksEventListeners() to setupPageNavigationListeners() to better reflect that it handles multiple navigation libraries, not just Turbolinks - Rename isEventListenerInitialized to isPageLifecycleInitialized for clarity - Extract hasNavigationLibrary variable to make the conditional logic clearer - Update debug messages to be more descriptive ("DETECTED" vs "USING") - Emphasize that the normal use case is NO navigation library These changes make the code more self-documenting and easier to understand, especially given that most users don't use Turbolinks at all. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix prettier formatting in CLAUDE.md 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent 43326ba commit 06f52f5

File tree

2 files changed

+27
-14
lines changed

2 files changed

+27
-14
lines changed

CLAUDE.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
55
## Development Commands
66

77
### Essential Commands
8+
89
- **Install dependencies**: `bundle && yarn`
9-
- **Run tests**:
10+
- **Run tests**:
1011
- Ruby tests: `rake run_rspec`
1112
- JavaScript tests: `yarn run test` or `rake js_tests`
1213
- All tests: `rake` (default task runs lint and all tests except examples)
@@ -18,58 +19,69 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
1819
- **Type checking**: `yarn run type-check`
1920

2021
### Development Setup Commands
22+
2123
- **Initial setup**: `bundle && yarn && rake shakapacker_examples:gen_all && rake node_package && rake`
2224
- **Prepare examples**: `rake shakapacker_examples:gen_all`
2325
- **Generate node package**: `rake node_package`
2426
- **Run single test example**: `rake run_rspec:example_basic`
2527

2628
### Test Environment Commands
29+
2730
- **Dummy app tests**: `rake run_rspec:dummy`
2831
- **Gem-only tests**: `rake run_rspec:gem`
2932
- **All tests except examples**: `rake all_but_examples`
3033

3134
## Project Architecture
3235

3336
### Dual Package Structure
37+
3438
This project maintains both a Ruby gem and an NPM package:
39+
3540
- **Ruby gem**: Located in `lib/`, provides Rails integration and server-side rendering
3641
- **NPM package**: Located in `node_package/src/`, provides client-side React integration
3742

3843
### Core Components
3944

4045
#### Ruby Side (`lib/react_on_rails/`)
46+
4147
- **`helper.rb`**: Rails view helpers for rendering React components
4248
- **`server_rendering_pool.rb`**: Manages Node.js processes for server-side rendering
4349
- **`configuration.rb`**: Global configuration management
4450
- **`engine.rb`**: Rails engine integration
4551
- **Generators**: Located in `lib/generators/react_on_rails/`
4652

4753
#### JavaScript/TypeScript Side (`node_package/src/`)
54+
4855
- **`ReactOnRails.ts`**: Main entry point for client-side functionality
4956
- **`serverRenderReactComponent.ts`**: Server-side rendering logic
5057
- **`ComponentRegistry.ts`**: Manages React component registration
5158
- **`StoreRegistry.ts`**: Manages Redux store registration
5259

5360
### Build System
61+
5462
- **Ruby**: Standard gemspec-based build
5563
- **JavaScript**: TypeScript compilation to `node_package/lib/`
5664
- **Testing**: Jest for JS, RSpec for Ruby
5765
- **Linting**: ESLint for JS/TS, RuboCop for Ruby
5866

5967
### Examples and Testing
68+
6069
- **Dummy app**: `spec/dummy/` - Rails app for testing integration
6170
- **Examples**: Generated via rake tasks for different webpack configurations
6271
- **Rake tasks**: Defined in `rakelib/` for various development operations
6372

6473
## Important Notes
74+
6575
- Use `yalc` for local development when testing with external apps
6676
- The project supports both Webpacker and Shakapacker
6777
- Server-side rendering uses isolated Node.js processes
6878
- React Server Components support available in Pro version
6979
- Generated examples are in `gen-examples/` (ignored by git)
7080

7181
## IDE Configuration
82+
7283
Exclude these directories to prevent IDE slowdowns:
84+
7385
- `/coverage`, `/tmp`, `/gen-examples`, `/node_package/lib`
7486
- `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp`
75-
- `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`
87+
- `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`

node_package/src/pageLifecycle.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,48 +28,49 @@ function runPageUnloadedCallbacks(): void {
2828
});
2929
}
3030

31-
function setupTurbolinksEventListeners(): void {
31+
function setupPageNavigationListeners(): void {
3232
// Install listeners when running on the client (browser).
33-
// We must do this check for turbolinks AFTER the document is loaded because we load the
33+
// We must check for navigation libraries AFTER the document is loaded because we load the
3434
// Webpack bundles first.
35-
if ((!turbolinksInstalled() || !turbolinksSupported()) && !turboInstalled()) {
36-
debugTurbolinks('NOT USING TURBOLINKS: calling reactOnRailsPageLoaded');
35+
const hasNavigationLibrary = (turbolinksInstalled() && turbolinksSupported()) || turboInstalled();
36+
if (!hasNavigationLibrary) {
37+
debugTurbolinks('NO NAVIGATION LIBRARY: running page loaded callbacks immediately');
3738
runPageLoadedCallbacks();
3839
return;
3940
}
4041

4142
if (turboInstalled()) {
42-
debugTurbolinks('USING TURBO: document added event listeners turbo:before-render and turbo:render.');
43+
debugTurbolinks('TURBO DETECTED: adding event listeners for turbo:before-render and turbo:render.');
4344
document.addEventListener('turbo:before-render', runPageUnloadedCallbacks);
4445
document.addEventListener('turbo:render', runPageLoadedCallbacks);
4546
runPageLoadedCallbacks();
4647
} else if (turbolinksVersion5()) {
4748
debugTurbolinks(
48-
'USING TURBOLINKS 5: document added event listeners turbolinks:before-render and turbolinks:render.',
49+
'TURBOLINKS 5 DETECTED: adding event listeners for turbolinks:before-render and turbolinks:render.',
4950
);
5051
document.addEventListener('turbolinks:before-render', runPageUnloadedCallbacks);
5152
document.addEventListener('turbolinks:render', runPageLoadedCallbacks);
5253
runPageLoadedCallbacks();
5354
} else {
54-
debugTurbolinks('USING TURBOLINKS 2: document added event listeners page:before-unload and page:change.');
55+
debugTurbolinks('TURBOLINKS 2 DETECTED: adding event listeners for page:before-unload and page:change.');
5556
document.addEventListener('page:before-unload', runPageUnloadedCallbacks);
5657
document.addEventListener('page:change', runPageLoadedCallbacks);
5758
}
5859
}
5960

60-
let isEventListenerInitialized = false;
61+
let isPageLifecycleInitialized = false;
6162
function initializePageEventListeners(): void {
6263
if (typeof window === 'undefined') return;
6364

64-
if (isEventListenerInitialized) {
65+
if (isPageLifecycleInitialized) {
6566
return;
6667
}
67-
isEventListenerInitialized = true;
68+
isPageLifecycleInitialized = true;
6869

6970
if (document.readyState === 'complete') {
70-
setupTurbolinksEventListeners();
71+
setupPageNavigationListeners();
7172
} else {
72-
document.addEventListener('DOMContentLoaded', setupTurbolinksEventListeners);
73+
document.addEventListener('DOMContentLoaded', setupPageNavigationListeners);
7374
}
7475
}
7576

0 commit comments

Comments
 (0)