Skip to content

always send on load even on going on the link#5469

Merged
adhami3310 merged 6 commits into
mainfrom
khaleel/eng-6299-regression-on_load-not-called-when-navigating-to-same-page
Jun 25, 2025
Merged

always send on load even on going on the link#5469
adhami3310 merged 6 commits into
mainfrom
khaleel/eng-6299-regression-on_load-not-called-when-navigating-to-same-page

Conversation

@adhami3310
Copy link
Copy Markdown
Member

fixes a regression when moving off of nextjs

@linear
Copy link
Copy Markdown

linear Bot commented Jun 18, 2025

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Modified state management to ensure on_load events are consistently triggered when navigating between pages, including navigation to the same page, fixing a regression introduced after moving away from NextJS.

  • Modified reflex/.templates/web/utils/state.js to remove pathname/search/hash comparison checks, ensuring route change events fire on every location change
  • Added integration test in tests/integration/test_dynamic_routes.py to verify on_load events trigger when navigating to the same static page multiple times

2 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile

Comment thread reflex/.templates/web/utils/state.js
Comment thread tests/integration/test_dynamic_routes.py
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 18, 2025

CodSpeed Performance Report

Merging #5469 will not alter performance

Comparing khaleel/eng-6299-regression-on_load-not-called-when-navigating-to-same-page (e592916) with main (9d5d59b)

Summary

✅ 8 untouched benchmarks

adhami3310 and others added 5 commits June 18, 2025 13:32
* memoize addEvents, which was being overwritten quite a bit
* avoid sending the onLoadInternal when the app initially loads (as it will
  send hydrate itself)
…ession-on_load-not-called-when-navigating-to-same-page
…ocation

Since the backend handles determining which on_load goes with the route, it
will pick the correct one the first time, whether that's a real dynamic route
or the actual 404 page.
@adhami3310 adhami3310 merged commit 38bcc06 into main Jun 25, 2025
41 checks passed
@adhami3310 adhami3310 deleted the khaleel/eng-6299-regression-on_load-not-called-when-navigating-to-same-page branch June 25, 2025 16:05
masenf added a commit that referenced this pull request Jun 26, 2025
* always send on load even on going on the link

* restore deps

* make it closer to what nextjs was doing

* avoid extra route change on_load

* memoize addEvents, which was being overwritten quite a bit
* avoid sending the onLoadInternal when the app initially loads (as it will
  send hydrate itself)

* Avoid second on_load when 404 client side routing redirects to same location

Since the backend handles determining which on_load goes with the route, it
will pick the correct one the first time, whether that's a real dynamic route
or the actual 404 page.

---------

Co-authored-by: Masen Furer <m_github@0x26.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants