Skip to content

Remove ControllerContext TMap, delete ihp-context package#2633

Open
mpscholten wants to merge 29 commits intomasterfrom
worktree-remove-controllercontext-tmap
Open

Remove ControllerContext TMap, delete ihp-context package#2633
mpscholten wants to merge 29 commits intomasterfrom
worktree-remove-controllercontext-tmap

Conversation

@mpscholten
Copy link
Copy Markdown
Member

Summary

After PRs #2632 and #2259 moved every consumer of the controller-context typed-map to the WAI request vault, the TMap inside ControllerContext was dead weight. This PR finishes the cleanup:

  • ControllerContext becomes a thin record wrapping the Request. All per-request state lives in request.vault.
  • Removes putContext / fromContext / maybeFromContext / fromFrozenContext / maybeFromFrozenContext / freeze / unfreeze / FrozenControllerContext.
  • AutoRefresh and Render no longer freeze/unfreeze — the captured request closure is the snapshot.
  • Deletes the legacy initAuthentication wrapper (use authMiddleware from PR Move auth to WAI request vault, remove unsafePerformIO from currentUserOrNothing #2259).
  • Deletes the entire ihp-context package (no longer pulling its weight; ihp-pagehead and ihp-modal were the original justification but neither imports anything from it).
  • Deletes Test.Controller.ContextSpec (tested the removed API).
  • Updates Guide and UPGRADE.md to use vault-key patterns instead of putContext/fromFrozenContext.

Breaking: any user code that calls putContext/fromContext/etc. or imports from ihp-context needs to migrate to dedicated Vault.Keys. The UPGRADE.md note has a worked example.

Test plan

  • All 475 IHP tests pass
  • All 400 IDE tests pass
  • All modified modules type-check
  • DataSync integration test passes in CI (nix flake check)

🤖 Generated with Claude Code

mpscholten and others added 6 commits April 13, 2026 11:06
ControllerContext is now a thin record wrapping the WAI Request:
all per-request state lives in request.vault. The TMap-based API
(putContext, fromContext, freeze, unfreeze, FrozenControllerContext)
is no longer used by the framework after PRs #2632 and #2259, so
remove it along with the ihp-context package.

Other changes that fall out of this:
- AutoRefresh and Render no longer freeze/unfreeze the context,
  the captured request closure is the snapshot.
- Drop the legacy initAuthentication wrapper (use authMiddleware).
- Delete Test.Controller.ContextSpec.
- Update Guide/UPGRADE docs to use vault-key patterns.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The new ControllerContext { request :: Request } auto-generated a
top-level 'request' selector that shadowed IHP.ControllerSupport.request
everywhere they were both in scope (e.g. Job/Dashboard.hs).

NoFieldSelectors suppresses the selector function while keeping
record construction syntax and HasField-based access (?context.request)
working.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
It has a single constructor with a single field, so the newtype
gives us zero runtime overhead over the data constructor.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mpscholten and others added 9 commits April 13, 2026 12:39
The ihp-forum master still uses the removed initAuthentication /
fromFrozenContext API. Point the core-size benchmark at the
migrate-to-auth-middleware branch until
digitallyinduced/ihp-forum#21 is merged.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After the TMap removal, ControllerContext is a trivial wrapper around
Request with HasField instances that just delegate back. Making it a
type alias (type ControllerContext = Request) removes the newtype
overhead, drops the delegating HasField instances, and simplifies
newControllerContext to 'pure ?request'.

Call sites that previously used '?context.request' now use '?context'
directly (same value). Tests that built ControllerContext via the
record constructor now return the Request they had already built.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Since ControllerContext is now a type alias for Request, the tests
that still annotated helper types with ControllerContext need to
either import it or switch to Request directly. Picked Request for
clarity.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…llerSupport

ControllerContext was a one-line type alias for Request and a trivial
'newControllerContext = pure ?request' wrapper. Inline the alias into
IHP.ControllerSupport (where the Controller class lives), drop the
separate module, and remove the prelude re-exports.

In the framework code, every type signature using ControllerContext is
mass-renamed to Request directly. ?context stays as an implicit param
(needed for Log/urlTo's polymorphic ConfigProvider/LoggingProvider
constraint), but is now plainly a Request.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mpscholten
Copy link
Copy Markdown
Member Author

Closing+reopening to re-trigger CI; workflow event seems stuck.

@mpscholten mpscholten closed this Apr 13, 2026
@mpscholten mpscholten reopened this Apr 13, 2026
mpscholten added a commit to digitallyinduced/ihp-zip that referenced this pull request Apr 13, 2026
ControllerContext is now re-exported from IHP.ControllerSupport
(see digitallyinduced/ihp#2633). The dedicated module is gone, so
remove the now-unnecessary import; the type comes through
IHP.ControllerSupport which is already imported.
…import

ihp-zip 0.1.0 (Hackage) imports IHP.Controller.Context which this PR
deletes. Pin to the digitallyinduced/ihp-zip#2 commit until 0.1.1 ships.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mpscholten added a commit to digitallyinduced/ihp-zip that referenced this pull request Apr 13, 2026
ControllerContext is now re-exported from IHP.ControllerSupport
(see digitallyinduced/ihp#2633). The dedicated module is gone, so
remove the now-unnecessary import; the type comes through
IHP.ControllerSupport which is already imported.
mpscholten and others added 3 commits April 13, 2026 18:26
ihp-zip 0.1.1 is now on Hackage with the fix for the removed
IHP.Controller.Context import, so we can use the regular Hackage-based
pin and remove the temporary fetchTarball override.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…quest

Since ControllerContext is a type alias for Request, having both
?context :: Request and ?request :: Request in a constraint set was
pure duplication. Keep ?request as the canonical name. Where function
bodies still need ?context in scope (for Log/urlTo/getAppConfig calls),
add a local 'let ?context = ?request' shim.

Top-level class constraints (Controller.action, View.html, etc.) keep
?context so user code calling Log/urlTo doesn't need to bind it
themselves.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Since ControllerContext is a type alias for Request, the ?context and
?request implicit parameters were carrying the same value. Rename
?context :: Request to ?request :: Request in framework function
signatures (and corresponding body references to ?request.field).

Polymorphic signatures (?context :: context with LoggingProvider /
ConfigProvider constraints) are left alone — those still use ?context
so jobs can bind 'let ?context = frameworkConfig' and call Log/urlTo.

Where a function body still needs ?context in scope (e.g. to call
Log.error / isEnvironment / getAppConfig), a local
'let ?context = ?request' shim is added.

Top-level View class constraint keeps ?context :: Request so that
user view code (which the framework renders via renderHtml) has
?context in scope for hsx route/asset helpers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mpscholten and others added 7 commits April 14, 2026 11:25
…t-implicit

Rename ?context :: Request to ?request :: Request in framework internals
…pe constraints

CI on the merged branch failed because buildBaseJobTable lost ?context
in its signature but its body calls renderBaseJobTable which still
needs it. Add the standard 'let ?context = ?request' shim.

Also dedupe (?request :: Request, ?modelContext, ?request :: Request)
patterns left over from the bulk rename across ihp-job-dashboard.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
setRLSConfigSession / sqlQueryWithRLSSession / sqlExecWithRLSSession
and friends call Role.authenticatedRole, which is polymorphic on
?context :: context with ConfigProvider. Their bodies use a where
clause, so a let-shim doesn't reach. Putting ?context :: Request
back in the constraint list is the cleanest fix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- runDataSyncController: add 'let ?context = ?request' shim
- buildMessageHandler: keep ?context :: Request (where bindings need it)
- maxTransactions/SubscriptionsPerConnection: shim for getAppConfig
  (polymorphic ConfigProvider call)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ext)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Core Size & Compile Allocations Benchmark

Metric Baseline (master) This PR Change
Core size 11014194 bytes 10983514 bytes -0.3%
Compile allocations 26996188216 bytes 27036962496 bytes 0.2%

Core size within threshold
Compile allocations within threshold

HTTP Latency (GET /, 5000 reqs, 10 concurrent)

Metric Baseline (master) This PR Change
Mean 3.25ms 3.42ms 5.2%
p50 3.10ms 3.30ms
p99 6.60ms 6.20ms
Min 0.60ms 0.60ms
Max 45.40ms 55.60ms
Req/s 2996 2885

HTTP latency within threshold

Top 10 modules (this PR)

Module Size (bytes)
Web.Types.thr 547347
Web.Routes.thr 423681
Web.View.Threads.Show.thr 304260
Web.Controller.Comments.thr 301525
Web.Controller.Threads.thr 291163
Web.FrontController.thr 274962
Admin.Types.thr 264263
build.Generated.User.thr 259919
Admin.Routes.thr 258465
Web.Controller.Users.thr 257529

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.

1 participant