search

fulcrologic / fulcro-rad

Fulcro Rapid Application Development
MIT License
201 stars 46 forks source link

Improve routing behavior during synchronous tx processing #109

Closed JJ-Atkinson closed 1 year ago

JJ-Atkinson commented 1 year ago

Revert order of push-route! and dr/change-route! back to the way it was. With the recent change to synchronous tx processing, routing is no longer async during tests. With some of our routing logic, will-enter causes a re-route to the sign-in page if the user has no credentials. That occurs within the dr/change-route! call, and current-route no longer matches new-route. Since push-route is called after change-route! without checking if the route has changed in the interim, the current-route fn on history implementations is updated to new-route and tests are messed up.

This is a closest approximation that restores both the original behavior of RAD before this commit and retains the :denied behavior. push-route! is called before dr/change-route, but denied? is still checked for. The fn can-change-route? is a partial extraction of dr/change-route!. An alternative implementation would be to split that function in fulcro itself, but that seems like a pretty major change and unlikely to go over well :).

awkay commented 1 year ago

Sure, that seems reasonable. Thanks for the patch.