fix(hooks): call hooks unconditionally before early returns (MAPPS-377) #442

Merged

nrupard merged 1 commit from fix/MAPPS-377-rules-of-hooks into main

2026-07-24 17:53:04 +02:00

nrupard commented

2026-07-24 17:06:30 +02:00

Owner

What

Fix Dioxus rules-of-hooks violations across the SPA: every component now calls all of its hooks unconditionally, before any conditional early return. Closes MAPPS-377.

Why

A hook called after an early return (an admin/finance permission gate, a server-outage gate, or a route-param gate) runs on some render paths but not others, so the component's hook count varies render to render. Dioxus 0.7 tolerates this today (its index-based hook model truncates a strict prefix), but it is a documented rules-of-hooks violation and a latent state-corruption / panic risk the moment the pre-return hook set stops being a prefix or Dioxus changes its hook model. This is the same defect class MAPPS-366 fixed for the layout shell. The compiler and clippy do NOT catch it (the code builds and the gate passes with the ordering wrong), so each site was audited by reading it.

Changes

Audited every component with a character-level scanner that distinguishes real component-level early returns from returns inside closures / async blocks (the naive "return precedes use_ textually" check over-reports because save/delete handlers return early inside their own closures). Found and fixed 31 components; the MAPPS-366 use_page_title hoist had already resolved several sites the issue's stale line numbers referenced.

Two behaviour-preserving techniques, chosen per site:

Child-extraction (mount the post-gate body as a new child component so its hooks run unconditionally within it, and it is never mounted for a gated-out user, so no fetch fires that did not fire before): the 16 admin-gated Settings CRUD pages (Scheduling, Time Tracking, Work Types, Task Statuses, Asset Types, Company Industries, Project Types, Payment Terms, Ticket Statuses/Priorities/Types/Queues/Categories, RMM Connections/Device Mappings/Alert Rules), the 4 finance-gated billing pages (Invoice list, Payment list, Tax Rate list, Payment Gateway config), the 2 finance-gated contracts pages (Contract list, Rate Card list), and the 2 finance-gated quotes pages (Quote list, Quote detail). AccountDeletedOverlay is also child-extracted, and this one is load-bearing: its use_effect hooks clear OIDC tokens and fire the logout redirect, so hoisting them above the not-deleted gate would have logged out every healthy session; the terminal body now mounts only once the account-deleted signal has flipped.

Hoisting (move the hook calls above the gate; each only reads a global signal or router/auth context and takes no post-return state, so behaviour is identical): TeamPage (use_server_reachable, use_can_mutate and the two revoke-dialog use_signal calls), TimesheetApprovalsPage (use_server_reachable, use_can_mutate above the manager gate), ContextFilterBanner (use_navigator above the no-filter / lookup-failed returns), ImportExportSettingsPage (use_auth above the admin gate), and ReportDetailPage (the report-view use_remote_resource above the report-builder gate; build_view is a pure no-op returning the default view for the builder type, ReportKind::Unsupported, so no network fires for that path). QuoteDetailPage additionally hoists use_can_mutate above its inner outage gate.

use_update_check (the App-root auto-reload hook) is a non-component hook that early-returned on dev builds before calling use_signal / use_future / use_effect; the gate is a compile-time constant so its hook count never actually varied at runtime, but it is fixed for correctness by calling all four hooks unconditionally and no-op-ing inside each when there is no baseline (the polling future returns immediately, the visibilitychange listener is not registered), preserving the exact dev/prod behaviour.

No behaviour or rendered output changes for any state; only hook ordering and child-component boundaries. No unrelated logic was refactored. Nothing was left unfixed.

Tests

Verified against the repo's real Docker gate (just pre-commit, rust-builder-glibc image, the same steps as .forgejo/workflows/check.yml): cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, cargo check --target wasm32-unknown-unknown, and cargo test --lib all pass. Also re-ran the audit scanner after the sweep: zero components call a hook after a component-level early return.

## What Fix Dioxus rules-of-hooks violations across the SPA: every component now calls all of its hooks unconditionally, before any conditional early `return`. Closes MAPPS-377. ## Why A hook called after an early `return` (an admin/finance permission gate, a server-outage gate, or a route-param gate) runs on some render paths but not others, so the component's hook count varies render to render. Dioxus 0.7 tolerates this today (its index-based hook model truncates a strict prefix), but it is a documented rules-of-hooks violation and a latent state-corruption / panic risk the moment the pre-return hook set stops being a prefix or Dioxus changes its hook model. This is the same defect class MAPPS-366 fixed for the layout shell. The compiler and clippy do NOT catch it (the code builds and the gate passes with the ordering wrong), so each site was audited by reading it. ## Changes Audited every component with a character-level scanner that distinguishes real component-level early returns from returns inside closures / async blocks (the naive "return precedes use_ textually" check over-reports because save/delete handlers return early inside their own closures). Found and fixed 31 components; the MAPPS-366 use_page_title hoist had already resolved several sites the issue's stale line numbers referenced. Two behaviour-preserving techniques, chosen per site: Child-extraction (mount the post-gate body as a new child component so its hooks run unconditionally within it, and it is never mounted for a gated-out user, so no fetch fires that did not fire before): the 16 admin-gated Settings CRUD pages (Scheduling, Time Tracking, Work Types, Task Statuses, Asset Types, Company Industries, Project Types, Payment Terms, Ticket Statuses/Priorities/Types/Queues/Categories, RMM Connections/Device Mappings/Alert Rules), the 4 finance-gated billing pages (Invoice list, Payment list, Tax Rate list, Payment Gateway config), the 2 finance-gated contracts pages (Contract list, Rate Card list), and the 2 finance-gated quotes pages (Quote list, Quote detail). AccountDeletedOverlay is also child-extracted, and this one is load-bearing: its use_effect hooks clear OIDC tokens and fire the logout redirect, so hoisting them above the not-deleted gate would have logged out every healthy session; the terminal body now mounts only once the account-deleted signal has flipped. Hoisting (move the hook calls above the gate; each only reads a global signal or router/auth context and takes no post-return state, so behaviour is identical): TeamPage (use_server_reachable, use_can_mutate and the two revoke-dialog use_signal calls), TimesheetApprovalsPage (use_server_reachable, use_can_mutate above the manager gate), ContextFilterBanner (use_navigator above the no-filter / lookup-failed returns), ImportExportSettingsPage (use_auth above the admin gate), and ReportDetailPage (the report-view use_remote_resource above the report-builder gate; build_view is a pure no-op returning the default view for the builder type, ReportKind::Unsupported, so no network fires for that path). QuoteDetailPage additionally hoists use_can_mutate above its inner outage gate. use_update_check (the App-root auto-reload hook) is a non-component hook that early-returned on dev builds before calling use_signal / use_future / use_effect; the gate is a compile-time constant so its hook count never actually varied at runtime, but it is fixed for correctness by calling all four hooks unconditionally and no-op-ing inside each when there is no baseline (the polling future returns immediately, the visibilitychange listener is not registered), preserving the exact dev/prod behaviour. No behaviour or rendered output changes for any state; only hook ordering and child-component boundaries. No unrelated logic was refactored. Nothing was left unfixed. ## Tests Verified against the repo's real Docker gate (`just pre-commit`, rust-builder-glibc image, the same steps as `.forgejo/workflows/check.yml`): `cargo fmt --all --check`, `cargo clippy --all-targets -- -D warnings`, `cargo check --target wasm32-unknown-unknown`, and `cargo test --lib` all pass. Also re-ran the audit scanner after the sweep: zero components call a hook after a component-level early return.

nrupard added 1 commit

2026-07-24 17:06:30 +02:00

fix(hooks): call hooks unconditionally before early returns

Check / fmt + clippy + tests (pull_request) Successful in 10m55s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

4374407297

Across 31 components a Dioxus hook (use_signal, use_resource, use_server_reachable, use_can_mutate, use_navigator, use_effect, use_remote_resource, and permission helpers that call use_auth) was invoked after a conditional early return, so the component's hook count varied render to render. Dioxus 0.7 tolerates the strict-prefix truncation today, but it is a documented rules-of-hooks violation and a latent state-corruption risk, and neither the compiler nor clippy flags it. Same defect class as MAPPS-366.

Every component now calls all of its hooks before any early return. Data-fetching permission gates (16 Settings CRUD pages, the billing / contracts / quotes finance-gated lists, Quote detail) mount their body as a new child component past the gate, so hooks run unconditionally within it and no fetch fires for a gated-out user. AccountDeletedOverlay is child-extracted the same way because its effects clear OIDC tokens and fire the logout redirect, which must not run on a healthy session. Global-signal and context reads (Team, Timesheet Approvals, ContextFilterBanner, Import & Export, Report detail, Quote detail outage gate) are hoisted above their gates. use_update_check calls its four hooks unconditionally and no-ops inside each on dev builds. Behaviour and rendered output are unchanged for every state.

Verified with the repo Docker gate (just pre-commit): fmt, clippy -D warnings, wasm32-unknown-unknown check, and 223 lib tests all pass.

#MAPPS-377

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>