Extract AutoRoute into optional ihp-autoroute package - #2662
Open
mpscholten wants to merge 12 commits into
Open
Conversation
Removes incidental `instance AutoRoute X` declarations from: - top-level `Main.hs` (DemoController smoke-test app) - `integration-test/Web/Routes.hs` (PostsController) - `ihp-sitemap/Test/SEO/Sitemap.hs` (PostController test fixture) - `ihp/Test/Test/Controller/CookieSpec.hs` - `ihp/Test/Test/Controller/AccessDeniedSpec.hs` - `ihp/Test/Test/Controller/NotFoundSpec.hs` - `ihp/Test/Test/MockingSpec.hs` - `ihp/Test/Test/ViewSupportSpec.hs` - `ihp/Test/Test/AutoRefreshSpec.hs` Each becomes a `[routes|XController …|]` block preserving the exact URL shape (`/test/...` prefix, matching what AutoRoute's `actionPrefixText` would have generated for these `Test.*` / `Web.*` / `Main` modules) and tightening the HTTP method to what the test or fixture actually invokes (GET for navigation, POST for form submits) rather than the looser `ANY`. This is preparation for extracting AutoRoute into the optional `ihp-autoroute` package — once nothing in the monorepo depends on AutoRoute, the extraction can land cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves the legacy AutoRoute typeclass and its query-string decoder out of `ihp` core into a new sibling package `ihp-autoroute`. New apps should use the `[routes|...|]` DSL from `IHP.Router.DSL`. Existing apps that still use `instance AutoRoute X` opt in by adding `ihp-autoroute` to their flake's `haskellPackages` and importing `IHP.AutoRoute` in their routes module. What moved: - `class AutoRoute` (autoRouteWithIdType, autoRoute, applyAction, allowedMethodsForAction, customRoutes, customPathTo) - query-string decoder (`parseFuncs`, `querySortedByFields`, `applyConstr`) - `routeMatchParser`, `stripActionSuffix*` - `createAction`, `updateAction` - overlappable `CanRoute` / `HasPath` instances for AutoRoute controllers - `class QueryParam`, `renderFieldForUrl` - `buildAutoRouteMap`, `parseUUIDOrTextId`, `parseRouteWithId` - the `TypeError` helper that catches missing constructor arguments What stays in core: - `class CanRoute`, `class HasPath`, `class FrontController` - `data ControllerRoute` (Map / Parser / Trie variants — `ControllerRouteMap` is now only produced by `ihp-autoroute`) - `data UnexpectedMethodException`, `data TypedAutoRouteError` (the latter so `ErrorController` can render typed 400 pages without depending on `ihp-autoroute`) - `actionPrefixText`, `wrapRouterException` — used by `startPage`, `catchAll`, and the error handler - `parseRoute`, dispatch helpers, all non-AutoRoute parsing helpers Tests: `ihp/Test/Test/RouterSupportSpec.hs` and `ihp/Test/Test/Router/MixedModeSpec.hs` move to `ihp-autoroute/Test/AutoRoute/`. The `Test/RouterBench.hs` benchmark moves to the same place (preserving the AutoRoute-vs-Trie comparison alongside its now-home). Nix: `ihp-autoroute` is registered in `NixSupport/overlay.nix` so apps can pick it up via `p.ihp-autoroute` in `haskellPackages`. Docs: `Guide/routing.markdown` now flags AutoRoute as opt-in, `UPGRADE.md` documents the two-step migration (`flake.nix` + `import`), and `CHANGELOG.md` lists this as a breaking change. Stacked on PR #2661 (ihp-ide DSL migration) so `ihp-ide` no longer relies on AutoRoute. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Repoint ihp benchmark target after moving RouterBench
The benchmark source has been moved to ihp-autoroute, but ihp/ihp.cabal still declares benchmark ihp-router-bench with main-is: Test/RouterBench.hs (line 331). That leaves the ihp benchmark component unbuildable because the referenced entrypoint was deleted from the ihp package.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,24 +1,25 @@ | |||
| {-| | |||
There was a problem hiding this comment.
Test.RouterSupportSpec from ihp tests
This spec was moved into ihp-autoroute, but ihp/ihp.cabal still lists Test.RouterSupportSpec in the test-suite tests other-modules section (line 311). As a result, building the ihp test component via Cabal/Nix will fail because the old module file no longer exists in ihp/Test/Test.
Useful? React with 👍 / 👎.
| import IHP.ViewPrelude | ||
| import IHP.ControllerPrelude hiding (get, request) | ||
| import IHP.Router.DSL (routes) | ||
| import IHP.Router.Capture (renderCapture, parseCapture) |
There was a problem hiding this comment.
parseCapture import in sitemap test
This routes block only uses a query parameter (?postId) and no path captures, so the splice needs renderCapture but not parseCapture. With -Werror=unused-imports enabled in ihp-sitemap, this explicit parseCapture import becomes a redundant-import error and can break the test build.
Useful? React with 👍 / 👎.
Without this entry, `nix flake check` doesn't build/test the new package and CI silently passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors what the codegen actually emits today (`[routes|...|]` block) and adds a callout pointing readers at the AutoRoute opt-in path for older projects. Other Guide pages still mention `instance AutoRoute X` inline; those continue to work as long as readers have `ihp-autoroute` in their flake (they will until upgrading) and can be modernized in follow-ups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Test.RouterSupportSpec` moved to `ihp-autoroute/Test/AutoRoute/` in the previous commit, and the `Test/RouterBench.hs` benchmark went with it, so `ihp`'s test suite and benchmark sections need to drop those references — otherwise `cabal build` (and `nix flake check`) can't find the modules. Regenerates ihp/default.nix to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec only emits paths via the [routes|...|] block (which uses renderCapture); it never decodes captures, so parseCapture is unused and trips -Werror=unused-imports under nix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Network.HTTP.Types already re-exports the StdMethod constructors, so the explicit Network.HTTP.Types.Method import was tripping -Werror=unused-imports under nix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
IHP.RouterSupport already re-exports ControllerRoute (..), so the explicit import from IHP.Router.Types was tripping -Werror=unused-imports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Was leftover from when the module was inside ihp-core; nothing in the extracted module references ModelSupport. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test-suite uses hs-source-dirs: . which recompiles IHP.AutoRoute.hs from scratch, so it needs the same deps as the library section (unordered-containers, uri-encode, blaze-html, mtl) plus Test.AutoRoute.Util in other-modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The TH splice (\$(pure [])) for the [routes|...|] block creates a declaration-group boundary. With main referencing tests at line 21 and tests defined at line 82 (after the splice), GHC couldn't resolve the forward reference across groups. Moving main to the end of the file fixes it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AutoRoutetypeclass and its query-string decoder out ofihpcore into a new sibling packageihp-autoroute.instance AutoRoute Xopt in by addingp.ihp-autorouteto their flake'shaskellPackagesandimport IHP.AutoRoutein their routes module.[routes|...|]DSL — no source change for them; the codegen already emits DSL.Stacked on #2661 (ihp-ide DSL migration). Merge that one first.
What moved to
ihp-autorouteclass AutoRoute(incl.autoRoute,autoRouteWithIdType,applyAction,allowedMethodsForAction,customRoutes,customPathTo)parseFuncs,querySortedByFields,applyConstr)routeMatchParser,stripActionSuffix*createAction,updateActionCanRoute/HasPathinstances for AutoRoute controllersclass QueryParam,renderFieldForUrlbuildAutoRouteMap,parseUUIDOrTextId,parseRouteWithIdTypeErrorhelper that renders "you forgot to pass an argument"Test/Test/RouterSupportSpec.hs,Test/Test/Router/MixedModeSpec.hs,Test/RouterBench.hs)What stays in
ihpcoreclass CanRoute,class HasPath,class FrontControllerdata ControllerRoute(Map/Parser/Trievariants —Mapis now only produced byihp-autoroute)data UnexpectedMethodException,data TypedAutoRouteError(kept soIHP.ErrorControllercan render typed 400s without depending onihp-autoroute)actionPrefixText,wrapRouterException— used bystartPage,catchAll, error handlerparseRoute, dispatch helpers, all non-AutoRoute parsing helpersMigration for apps
Two-step:
p.ihp-autoroutetoihp.haskellPackagesinflake.nix.import IHP.AutoRoutewherever you haveinstance AutoRoute X.URL shapes,
pathTo, HTTP-method dispatch all behave identically to before. Documented inUPGRADE.mdandCHANGELOG.md.Test plan
ihptest suite (513 examples, 0 failures, 23 pending without DB) viaecho -e ':l ihp/Test/Test/Main.hs\nmain' | ghciihp-autoroutetest suite (48 examples, 0 failures) viaecho -e ':l ihp-autoroute/Test/Main.hs\nmain' | ghci— covers the movedRouterSupportSpec(typed query-string decoding) andMixedModeSpec(AutoRoute + DSL coexist)ihp-idetest suite (401 examples, 0 failures)nix flake check) passes on this branchp.ihp-autoroute+import IHP.AutoRoute+instance AutoRoute SomeController, verify/Posts,/ShowPost?postId=…, etc. routes still resolve andpathTostill produces the same URLs.🤖 Generated with Claude Code