Code generation and delegation to nested routes#1887
Merged
parsonsmatt merged 191 commits intoJun 16, 2026
Conversation
Co-authored-by: Matt Parsons <parsonsmatt@gmail.com>
…ontent Show truncated content in handler content
…s-generation Mattp/control resources generation
parsonsmatt
commented
Jun 10, 2026
parsonsmatt
left a comment
parsonsmatt
left a comment
Collaborator
Author
There was a problem hiding this comment.
Woof. Huge PR. Sorry about that.
This PR is pretty big. But a huge component of that is a pretty good bit of new tests that exercise backwards compatibility and demonstrate behavior.
Comment on lines
+38
to
+45
| -- | This class enables you to dispatch on a route fragment without needing | ||
| -- to know how to dispatch on the entire route structure. This allows you | ||
| -- to break up route generation into multiple files. | ||
| -- | ||
| -- For details on use, see 'setFocusOnNestedRoute'. | ||
| -- | ||
| -- @since 1.7.0.0 | ||
| class RenderRouteNested a => YesodDispatchNested a where |
Collaborator
Author
There was a problem hiding this comment.
This is the really important addition here. We can give instances of YesodDispatchNested to sub-route fragments, and that can be consumed in hspec-yesod to allow tests to only depend on the actual sub-route handlers that they need.
Comment on lines
+185
to
+224
| toWaiAppYreNested | ||
| :: (Yesod (ParentSite a), YesodDispatchNested a, ToParentRoute a) | ||
| => Proxy a | ||
| -> ParentArgs a | ||
| -> YesodRunnerEnv (ParentSite a) | ||
| -> W.Application | ||
| toWaiAppYreNested proxy parentArgs yre req = | ||
| case cleanPath site $ W.pathInfo req of | ||
| Left pieces -> sendRedirect site pieces req | ||
| Right pieces -> do | ||
| let mapplication = | ||
| yesodDispatchNested proxy parentArgs (toParentRoute parentArgs) yre req | ||
| { W.pathInfo = pieces | ||
| } | ||
| case mapplication of | ||
| Nothing -> | ||
| yesodRunner (notFound :: HandlerFor site ()) yre Nothing req | ||
| Just k -> | ||
| k | ||
| where | ||
| site = yreSite yre | ||
| sendRedirect :: Yesod master => master -> [Text] -> W.Application | ||
| sendRedirect y segments' env sendResponse = | ||
| sendResponse $ W.responseLBS status | ||
| [ ("Content-Type", "text/plain") | ||
| , ("Location", BL.toStrict $ toLazyByteString dest') | ||
| ] "Redirecting" | ||
| where | ||
| -- Ensure that non-GET requests get redirected correctly. See: | ||
| -- https://github.com/yesodweb/yesod/issues/951 | ||
| status | ||
| | W.requestMethod env == "GET" = status301 | ||
| | otherwise = status307 | ||
|
|
||
| dest = joinPath y (resolveApproot y env) segments' [] | ||
| dest' = | ||
| if S.null (W.rawQueryString env) | ||
| then dest | ||
| else dest `mappend` | ||
| byteString (W.rawQueryString env) |
Collaborator
Author
There was a problem hiding this comment.
This implementation is pretty much the same as the prior toWaiAppYre - the main difference being that we call yesodDispatchNested instead of yesodDispatch. Notably, for a Route Site, this is exactly equivalent.
Comment on lines
-224
to
+212
| let appCxt = fmap (\ctxs -> | ||
| case ctxs of | ||
| c:rest -> | ||
| foldl' (\acc v -> acc `AppT` fst (nameToType v)) (ConT $ mkName c) rest | ||
| [] -> error $ "Bad context: " ++ show ctxs | ||
| ) appCxt' | ||
| appCxt <- buildAppCxt appCxt' |
Collaborator
Author
There was a problem hiding this comment.
bit of a drive-by refactor to use fail instead of error + extract the shared logic into a helper.
Comment on lines
+6
to
+15
| module Yesod.Routes.TH.Dispatch | ||
| ( MkDispatchSettings (..) | ||
| , mkDispatchClause | ||
| , defaultGetHandler | ||
| , SDC(..) | ||
| , mkDispatchInstance | ||
| , mkNestedDispatchInstance | ||
| , mkNestedSubDispatchInstance |
Collaborator
Author
There was a problem hiding this comment.
This module is pretty much where the worst of the dragons be.
Comment on lines
+60
to
+77
| , mdsNestedRouteFallthrough :: !Bool | ||
| -- ^ When 'True', fall through if no route matches (except in the final | ||
| -- case). When 'False', return 404 if the current route clause fails to | ||
| -- match. | ||
| -- | ||
| -- @since 1.7.0.0 | ||
| , mdsNestedDispatchClass :: Name | ||
| -- ^ The class to check for nested dispatch delegation. | ||
| -- @''YesodDispatchNested@ for top-level dispatch, | ||
| -- @''YesodSubDispatchNested@ for subsite dispatch. | ||
| -- | ||
| -- @since 1.7.0.0 | ||
| , mdsNestedDispatchFn :: Name | ||
| -- ^ The function to call for nested dispatch delegation. | ||
| -- @'yesodDispatchNested@ for top-level dispatch, | ||
| -- @'yesodSubDispatchNested@ for subsite dispatch. | ||
| -- | ||
| -- @since 1.7.0.0 |
Collaborator
Author
There was a problem hiding this comment.
These are essentially the three biggest things- supporting route fallthrough (necessary to support lots of route separation), allowing us to dispatch to a separate class, and the dispatch function name.
Comment on lines
+517
to
+526
| mkDispatchInstance routeOpts master cxt tyargs unwrapper res = | ||
| -- Branch on the focus target via the accessor rather than a constructor | ||
| -- pattern, so 'RouteOpts' can stay abstract (its constructor is not | ||
| -- exported). 'Just target' focuses a single nested route for | ||
| -- module-splitting; 'Nothing' generates the full top-level instance. | ||
| case roFocusOnNestedRoute routeOpts of | ||
| Just target -> | ||
| mkNestedDispatchInstance routeOpts target master cxt tyargs unwrapper res | ||
| Nothing -> | ||
| mkTopLevelDispatchInstance routeOpts master cxt tyargs unwrapper res |
Collaborator
Author
There was a problem hiding this comment.
Here's the split- if we're focusing, we filter and dispatch. Otherwise we carry on as usual.
Comment on lines
+74
to
+78
| -- | Look up a type by 'Name' and return it fully applied with fresh | ||
| -- type variables. This is needed because nested route datatypes may | ||
| -- have type parameters (e.g., @NestedR subsite@), and TH functions | ||
| -- like 'isInstance' require fully-applied types. | ||
| fullyApplyType :: Name -> Q Type |
Collaborator
Author
There was a problem hiding this comment.
hmm. I would be surprised if isInstance found an instance C (T Int) if it was looking for isInstance ''C =<< [t|T a|].
Comment on lines
+74
to
+78
| -- | Look up a type by 'Name' and return it fully applied with fresh | ||
| -- type variables. This is needed because nested route datatypes may | ||
| -- have type parameters (e.g., @NestedR subsite@), and TH functions | ||
| -- like 'isInstance' require fully-applied types. | ||
| fullyApplyType :: Name -> Q Type |
Collaborator
Author
There was a problem hiding this comment.
Claude commented below pretty confidently that I indeed would be surprised: https://github.com/yesodweb/yesod/pull/1887/changes#diff-f8cfe8596f2672887a201a293b33809bbd7e28f0bb58867b2fff6889ef348822R145-R157
Which makes sense, in hindsight. reifyInstances ''C =<< [t| T a |] should list out the instance C (T Int), and if it's a isInstance className typeArgs = not . null <$> reifyInstances className typeArgs then we'll get it here.
Comment on lines
+1
to
+10
| {-# language TemplateHaskell #-} | ||
| {-# language ViewPatterns, OverloadedStrings #-} | ||
| {-# language TypeFamilies #-} | ||
|
|
||
| -- | @Hierarchy@ split fragment focused on the deepest nested parent | ||
| -- @NestInner@ (under @NestR@ > @Nest2@). Emits its render\/attrs\/parse | ||
| -- instances via @setFocusOnNestedRoute "NestInner"@ + the @*For@ helpers; the | ||
| -- @ParseRouteNested NestInner@ instance is what "Hierarchy.Nest2" asserts | ||
| -- against at compile time. | ||
| module Hierarchy.Nest2.NestInner where |
Collaborator
Author
There was a problem hiding this comment.
So here starts the tests- by far the largest part of this PR.
I had Claude generate tests that would cover every interaction of feature: parameterized subsites, nested routes, separate compilation, fallthrough, etc.
Comment on lines
+20
to
+22
| mkRenderRouteInstanceOpts (setFocusOnNestedRoute "NestR" defaultOpts) [] NoTyArgs (ConT ''Hierarchy) hierarchyResourcesWithType | ||
| mkRouteAttrsInstanceFor [] (ConT ''NestR) "NestR" $ hierarchyResourcesWithType | ||
| mkParseRouteInstanceFor "NestR" $ hierarchyResourcesWithType |
Collaborator
Author
There was a problem hiding this comment.
This is really not how the public API should be used- it's internal details. mkYesod and mkYesodData/Dispatch are the better ways to go.
Contributor
There was a problem hiding this comment.
If so, it would be good to mention that; the moment this PR merges these are the canonical ways to use yesod
Collaborator
Author
There was a problem hiding this comment.
I modified the tests to use the more standard approach.
RenderRoute.hs: TemplateHaskellQuotes does not enable nested $(...) splices inside brackets on GHC 8.8/8.10 — they parse as the ($) operator and fail with cross-stage Lift errors. Use the full TemplateHaskell pragma. ParseRoute.hs: brackets are monomorphic Q before template-haskell 2.17, which conflicts with buildInlineParseClauses' Quote polymorphism. Keep the bracket but run it through th-compat's unsafeQToQuote, which is safe because the bracket draws no hygiene names (its only binder comes from a splice). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
L0neGamer
reviewed
Jun 11, 2026
Contributor
There was a problem hiding this comment.
I've given a look at everything but the tests.
Generally very good; I can see what's happening roughly and why it's happening.
There are a fair amount of refactors here that I'd have liked to see split out to reduce on noise, but with >100 commits I presume that would be very hard.
I'll review the tests tomorrow, but I'd really like it if there were some way to run the non-nested-based tests on the pre-this PR yesod state. If we could do that then we would be able to confidently say that there haven't been any breakage.
To combine those two thoughts: there's a lot of movement that isn't neccessarily strictly nested-dispatch related, which means that the new tests aren't trivially easy to run. Moving the non-feature related changes and tests to earlier in the commit history, or somehow separating them out, would increase my confidence in the lack of changes to functionality.
Comment on lines
+1033
to
+1034
| parseYesodName :: String -> Either String (String, [String], [[String]]) | ||
| parseYesodName name = do |
Contributor
There was a problem hiding this comment.
I don't think we should expose this from yesod if we can help it; if it isn't exposed, great, we can remove the docstring and maybe move it to the one module it's used in.
Comment on lines
84
to
89
| data SDC = SDC | ||
| { clause404 :: Clause | ||
| , extraParams :: [Exp] | ||
| { extraParams :: [Exp] | ||
| , extraCons :: [Exp] | ||
| , envExp :: Exp | ||
| , reqExp :: Exp | ||
| } |
Contributor
There was a problem hiding this comment.
I know you didn't make it but what is SDC? A comment here would be useful.
Collaborator
Author
There was a problem hiding this comment.
Oh god. I knew this at some point. SingleDispatchClause maybe? SomeDataCollection? 😅
Comment on lines
+182
to
+185
| -- @since 1.7.0.0 — the leading @[Name]@\/@[Exp]@ parameters were dropped and the | ||
| -- result type changed from @Clause@ to @Q ([String], Clause)@. | ||
| mkDispatchClause :: forall a b site c. TyArgs -> MkDispatchSettings b site c -> [ResourceTree a] -> Q ([String], Clause) | ||
| mkDispatchClause tyargs MkDispatchSettings {..} resources = do |
Contributor
There was a problem hiding this comment.
The previous version did not have [Name] or [Exp] as input as far as I can see.
Comment on lines
+68
to
+77
| -- | The number of type parameters a type constructor declares (0 for a type | ||
| -- that isn't a data\/newtype\/type-synonym, or that we can't reify). | ||
| typeArity :: Name -> Q Int | ||
| typeArity typeName = do | ||
| info <- reify typeName | ||
| pure $ case info of | ||
| TyConI (DataD _ _ vs _ _ _) -> length vs | ||
| TyConI (NewtypeD _ _ vs _ _ _) -> length vs | ||
| TyConI (TySynD _ vs _) -> length vs | ||
| _ -> 0 |
Contributor
There was a problem hiding this comment.
Out of interest, why not fail here if it's not an appropriate type constructor?
| Just tyname -> do | ||
| nestedArity <- typeArity tyname | ||
| maybe (pure ()) (fail . arityMismatchMessage callSite) $ | ||
| checkNestedSubArity subName (RouteName (rcName rc)) subArity (RouteArity nestedArity) |
Contributor
There was a problem hiding this comment.
I don't know the full context of the use of this function but I'm itching to say parse don't validate; are we able to move this sort of assertiong to RouteCon generation somehow?
Comment on lines
+306
to
+315
| -- 'unsafeQToQuote' because brackets are monomorphic 'Q' | ||
| -- on template-haskell < 2.17. It is safe here: the only | ||
| -- effect a bracket performs is drawing hygiene names for | ||
| -- its binders, and this one has none (the lambda binder | ||
| -- comes from a splice). | ||
| expr <- unsafeQToQuote [e| | ||
| fmap | ||
| (\ $(varP subName) -> $(pure wrapSub) ) | ||
| (parseRoute ( $(varE restName), $(varE queryParamsName) ) ) | ||
| |] |
Contributor
There was a problem hiding this comment.
instead of using the unsafe, would it make sense to just purely build the expression?
Collaborator
Author
There was a problem hiding this comment.
We could, but direct constructor use is a common pain point with upgrading TH versions. Quotes are a lot safer.
| -- import App.Routes.Resources | ||
| -- import Yesod.Core | ||
| -- | ||
| -- mkYesodOpts (setFocusOnNestedRoute "NestR" defaultOpts) "App" appResources |
Contributor
There was a problem hiding this comment.
so this example would generate a NestR type with two constructors, NestIndexR and NestShowR?
These wouldn't have the usual yesod instances but would have Nested instances which the App module would use when it makes the full application.
Contributor
There was a problem hiding this comment.
This is for my understanding of the general situation.
In normal yesod execution, I'm pretty sure we would make a top level NestR type with those constructors, so I guess the entire point here is that you can split up that specific generation.
Comment on lines
+239
to
258
| -- | If True, generate nested-discovery code (separate subroute datatypes and | ||
| -- @RenderRouteNested@ / dispatch instances) for a /parameterized/ site instead | ||
| -- of the backwards-compatible inline output. The subroute datatypes then carry | ||
| -- the parent site's type variables so that the @ParentSite@ \/ @ParentArgs@ | ||
| -- associated types stay well-scoped. | ||
| -- | ||
| -- Because the generated nested instances are parameterized over the site's | ||
| -- type variables, the splice emits instance heads with non-variable arguments; | ||
| -- a module using this typically needs @FlexibleContexts@, @FlexibleInstances@, | ||
| -- @MultiParamTypeClasses@, @TypeFamilies@ and, for a parameterized subsite | ||
| -- whose @master@ is determined by the @subsite@ (a @subsite -> master@ | ||
| -- functional dependency on the user's class), @UndecidableInstances@. | ||
| -- | ||
| -- Monomorphic sites always use nested discovery regardless of this flag; see | ||
| -- 'discoveryMode'. | ||
| -- | ||
| -- Default: 'False'. | ||
| -- | ||
| -- @since 1.6.28.0 | ||
| setParameterizedSubroute :: Bool -> RouteOpts -> RouteOpts |
Contributor
There was a problem hiding this comment.
I'm not sure this docstring is useful, or it might be too useful.
It mentions nesting types a lot but this feature functions regardless of nested routes, at least as I understand it.
| ( module Yesod.Routes.TH.Types | ||
| -- * Functions | ||
| , module Yesod.Routes.TH.RenderRoute | ||
| , -- ** RenderRoute |
Contributor
There was a problem hiding this comment.
explicit exports are very good!
Comment on lines
24
to
25
| , module Yesod.Routes.TH.ParseRoute | ||
| , module Yesod.Routes.TH.RouteAttrs |
Contributor
There was a problem hiding this comment.
are these worth being explicit about as well?
…covery # Conflicts: # yesod-core/test/YesodCoreTest.hs # yesod-core/test/YesodCoreTest/ZeroPieceShadow/ShadowApp.hs # yesod-core/test/YesodCoreTest/ZeroPieceShadowRuntime.hs # yesod-core/yesod-core.cabal
Collaborator
Author
|
I had Claude extract the backwards compatibility tests to #1919 at least on master, which should give us some confidence that things are working the same. |
L0neGamer
reviewed
Jun 12, 2026
L0neGamer
left a comment
L0neGamer
left a comment
Contributor
There was a problem hiding this comment.
The tests seem good.
It might assist readability and navigation if the Runtime and actual tests lived in the same folders as the setup data; as it stands it's not 100% clear when two modules relate to each other that easily.
If you're happy with waiting I'll do one last read through on monday.
Comment on lines
+51
to
+53
| -- /trailing-nest TrailingNestR: | ||
| -- /foo TrailingFooR GET | ||
| -- /#Int TrailingIntR GET |
Contributor
There was a problem hiding this comment.
These might still be needed to exercise a parse bug
Comment on lines
+20
to
+22
| mkRenderRouteInstanceOpts (setFocusOnNestedRoute "NestR" defaultOpts) [] NoTyArgs (ConT ''Hierarchy) hierarchyResourcesWithType | ||
| mkRouteAttrsInstanceFor [] (ConT ''NestR) "NestR" $ hierarchyResourcesWithType | ||
| mkParseRouteInstanceFor "NestR" $ hierarchyResourcesWithType |
Contributor
There was a problem hiding this comment.
If so, it would be good to mention that; the moment this PR merges these are the canonical ways to use yesod
Comment on lines
+40
to
+46
| it "rejects when the nested datatype carries more params than the subsite" $ | ||
| -- Over-arity leaves the instance head partially applied (kind | ||
| -- @Type -> ...@), so this must be rejected, not silently accepted. | ||
| check "MySub" "NestedR" 1 2 `shouldSatisfy` isJust | ||
|
|
||
| it "rejects a parameterized subsite with an unparameterized nested datatype" $ | ||
| check "MySub" "NestedR" 1 0 `shouldSatisfy` isJust |
Contributor
There was a problem hiding this comment.
do we want to assert the error message?
Comment on lines
+73
to
+83
| describe "parseRoute" $ do | ||
| let routeShouldParse path result = | ||
| parseRoute (path, []) `shouldBe` Just result | ||
| routeShouldNotParse path = | ||
| parseRoute (path, []) `shouldBe` (Nothing :: Maybe (Route App)) | ||
| it "can fall through" $ do | ||
| routeShouldParse ["foo", "blah"] (SecondFooR FooBlahR) | ||
| it "nested fallthrough works too" $ do | ||
| routeShouldParse ["foo", "baz", "foo"] (SecondFooR (FooBaz2R FooBaz2FooR)) | ||
| it "can fail" $ do | ||
| routeShouldNotParse ["asdf"] |
Contributor
There was a problem hiding this comment.
just for completeness would be nice to have a case for /foo
Comment on lines
+53
to
+54
| show FocusIndexR `shouldBe` "FocusIndexR" | ||
| show (FocusShowR 7) `shouldBe` "FocusShowR 7" |
Contributor
There was a problem hiding this comment.
Is there a way we can assert that these are the only constructors of FocusNestR?
Collaborator
Author
There was a problem hiding this comment.
Yeah I think a case expr ought to do it
|
|
||
| data App = App | ||
|
|
||
| -- TODO: make parseRoutesNoCheck smarter |
Comment on lines
+59
to
+61
| -- The headline R10 case. The top module wants fallthrough; FirstFooR | ||
| -- has no "other" child. Post-R10 this falls through to SecondFooR's | ||
| -- OtherR (200). Pre-R10, FirstFooR's split instance (flag False) bakes |
| -- separately compiled "YesodCoreTest.ParamDefaultSplitRuntime", so the child | ||
| -- datatype is in scope there — the precise shape that made the @go@ delegation | ||
| -- probe build the ill-kinded @SubParentDR a@ and abort the splice. | ||
| module YesodCoreTest.ParamDefaultSplitData where |
Contributor
There was a problem hiding this comment.
this breaks the general pattern you set of having the Data module in a subfolder, jsyk.
I think I'd really like a refactor of the tests so that the Runtimes are in the same folder as the Data's; it makes more sense where the tests live in that case.
Might even be worth renaming to Spec, but that can come in a future PR.
Comment on lines
+47
to
+58
| $(do | ||
| let subsite = mkName "subsite" | ||
| master = mkName "master" | ||
| cxt = [ ConT (mkName "PClass") `AppT` VarT subsite `AppT` VarT master ] | ||
| tyargs = SomeTyArgs ((VarT subsite, subsite) :| []) | ||
| mkNestedSubDispatchInstance | ||
| (setParameterizedSubroute True defaultOpts) | ||
| "PNestedR" | ||
| cxt | ||
| tyargs | ||
| return | ||
| (map (fmap (parseType . dropBracket)) resourcesPSub)) |
Contributor
There was a problem hiding this comment.
why the manual building?
| (["first", "1", "blah"], []) -> | ||
| pure @IO () | ||
| wrong -> | ||
| fail $ "Expecte renderRoute to work, but got: " <> show wrong |
Layout: runtime/spec modules now live in the same folder as their setup-data modules. Each multi-module test family is one directory (NestedDispatch/, SplitSubsite/, FallthroughMatrix/, FallthroughDispatch/, ParamDefaultSplit/, ParamFocusSplit/, ParamSubsite/, SubsiteFallthrough/, SubsiteOptsFallthrough/) with Data/Resources alongside Runtime. Self-contained single-module specs stay at the top level. Spec additions: - FallthroughSpec: parseRoute case for /foo (first parent matches without falling through) - SubDispatchAritySpec: assert the rendered error messages, not just isJust (one full-message golden, one content check) - FocusLeafConsSpec: assert the generated datatype's complete constructor set (names extracted in the generating splice; a later splice cannot reify a same-module spliced datatype) - InstanceProbeSpec: arity-2 abstract-instance case, and an instance-at-concrete-argument case (a unifier counts as could-match); note on why an unapplied-constructor instance is unrepresentable Comment cleanups: internal review labels (R10, T5, ISSUE 11, '#1 regression') rewritten as behavior descriptions; reviewer-relative framing neutralized; commented-out trailing-nest routes (inherited from the old Hierarchy.hs) dropped; stale TODO removed; 'Expecte' typo; ParamSplitSubNested now explains why the fragment entry point takes structured Cxt/TyArgs instead of a name string; Hierarchy.Nest documents the canonical low-level split recipe and points at the guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
L0neGamer
reviewed
Jun 12, 2026
Comment on lines
+340
to
+346
| -- A child clause matches the @(path, queryParams)@ tuple with no @where@ | ||
| -- bindings, so it converts directly to a @case@ alternative. (Assert the | ||
| -- shape we rely on rather than silently dropping non-empty @where@ decls.) | ||
| clauseToMatch :: Clause -> Match | ||
| clauseToMatch (Clause [pat] body []) = Match pat body [] | ||
| clauseToMatch c = | ||
| error $ "buildInlineParseClauses: unexpected child clause shape: " <> show c |
Contributor
There was a problem hiding this comment.
would moving this over to using MonadFail make more sense than using error here? Alternative would be don't match on Clause, and instead make buildInlineParseClauses' which gives the singular pattern and body that this expects.
| { extraParams = extraParams sdc ++ dyns | ||
| , extraCons = extraCons sdc ++ [constr] | ||
| } | ||
| , envWrap = justE |
Contributor
There was a problem hiding this comment.
Like I mentioned with some of the other things, since envWrap can only either be id or justE, would it make sense to use an enum/Bool to clearly label that this is either 'Just or it's nothing at all?
parsonsmatt
added a commit
that referenced
this pull request
Jun 15, 2026
Extracted from #1887 (split route compilation). These tests assert behavior that yesod-core already has on master: - ParameterizedSubDispatchRuntime: end-to-end dispatch through a parameterized subsite (associated types, ConstraintKinds, dynamic parent pieces), including 404s and method mismatches - MultiPieceNestedRuntime: nested parents whose prefix captures dynamics (/shop/#Int/admin), forwarding parent+child captures - ZeroPieceShadowRuntime: a zero-piece nested parent (/) shadows later siblings - longstanding, previously unpinned behavior - BangSeparatorRuntime: /!#Int dispatches like /#Int - RuntimeHarness: shared WAI round-trip assertion helper Running them against master first shows the feature branch preserves all of this; #1887 will be rebased on top so its diff no longer carries these files. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ParseRoute: split buildInlineParseClauses into a thin Match->Clause wrapper over a new buildInlineParseMatches that yields [Match] directly, so a parent folds its children's alternatives straight into its case. Removes the partial clauseToMatch and its error fallback (an unreachable shape assertion that a reader still had to prove dead). Dispatch: replace the envWrap :: Exp -> Exp field with envPhase :: DispatchPhase (TopLevelPhase | NestedPhase) interpreted by a single pure wrapForPhase. The field previously admitted any Exp -> Exp though only id / Just-wrap were ever valid; the enum makes the two phases the only inhabitants. One askWrap helper feeds both the leaf and parent bodies. Both are the tag-over-function / total-by-construction direction already taken with SameSpliceNestedInstances. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Parse.hs: pieceFromString is now MonadFail and routes Dynamic pieces
through dropBracketM, so an unclosed bracket in a route piece (e.g.
"#{Foo") surfaces as an attributed splice error instead of bottoming
out in the pure dropBracket's error. Closes the gap left by the
earlier piecesFromString MonadFail conversion.
- RenderRoute.hs: discoveryMode now takes TyArgs directly and calls
hasTyArgs internally, instead of a bare Bool. Every call site passed
`hasTyArgs tyargs` anyway; this removes the footgun the haddock had to
warn about (don't confuse the Bool with NoTyArgs). Callers updated in
RenderRoute, Dispatch, ParseRoute, Internal/TH; test updated to pass
TyArgs (toTyArgs []/[..]).
- RenderRoute.hs: delete commented-out pre-bracket delegatingBody code.
- Class/Dispatch.hs: UrlToDispatch haddock no longer implies the url
value is dispatched on; document that dispatch is path-based and the
url argument only selects the instance / carries the constraint.
- Routes/Class.hs: document that parseRouteNested and renderRouteNested
are deliberately not inverses (parse recovers only the fragment, never
the ParentArgs).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hand-written subsite-splitting recipe previously forced users to convert the quasi-quoter's [ResourceTree String] to [ResourceTree Type] themselves via `map (fmap (parseType . dropBracket))`, which meant re-exporting the *partial* parseType/dropBracket from the blessed Yesod.Core.Dispatch and running them inside the splice (a malformed type bottomed out as a GHC panic). Push the parse boundary inward: the public mkNestedSubDispatchInstance now takes [ResourceTree String] and calls parseResourceTypes internally (dropBracketM >=> parseTypeM, in Q), so a bad type fails the splice with an attributable error and users never touch the partial primitives. Because parsing lives in Yesod.Routes.Parse (above Yesod.Routes.TH.Dispatch in the import graph), the String-taking entry point lives in Yesod.Core.Internal.TH, delegating to the now-exported Type-taking worker mkNestedDispatchInstanceWith. The internal mkYesod path calls that worker directly with already-parsed resources (no double parse). Dropped the parseType/dropBracket re-exports from Yesod.Core.Dispatch. Recipe in docs + the three split-subsite test modules updated to pass the String resources directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mkNestedDispatchInstanceWith only arity-checks nested children, so the hand-written subsite-splitting recipe (mkNestedSubDispatchInstance -> worker) built its top target's instance head with no arity guard. A recipe pairing a parameterized subsite with an unparameterized target datatype would apply the subsite's type args to a kind-Type head and hit a cryptic kind error instead of the actionable arity message. Close the gap in the recipe wrapper, where target + tyargs are known: resolveRouteCon + assertNestedSubArity before delegating. No-op when the datatype is out of scope (unknowable arity) or the arities match, so the existing valid recipes (incl. the parameterized arity-1 case) are unaffected. On the parse-don't-validate angle: a ValidatedSubHead token threaded through the worker was considered but rejected. The arity message needs a "container" name (foundation subsite vs parent route) that differs per call site and lives in the caller, not at the single head-construction point, so a global token would degrade the message. The check is already un-ignorable in practice: checkNestedSubArity (pure, Maybe-returning) is called only by assertNestedSubArity (Q (), for effect); checkNestedSubArity stays exported solely for its unit test. A negative test (mismatched top target -> splice failure) isn't expressible without a compile-fail harness; the pure logic is covered by SubDispatchAritySpec and the happy-path wiring by the recipe modules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tidy warts surfaced by a -Wall + -Wunused-imports sweep of the branch's changed modules (the package builds with -Wall, so these would warn on a clean recompile): - RenderRoute: remove four dead imports (Yesod.Core.Class.Dispatch, Yesod.Core.Handler, Data.Proxy, Yesod.Core.Class.Yesod) — referenced only in haddock prose, never as code; and delete nullifyWhenNoParam, an exported helper with zero remaining call sites (the discoveryMode/ childCxt rewrite replaced it). Drop its re-export from Routes/TH too. - RenderRoute: drop the unused RouteOpts parameter from the internal mkToParentRouteInstances (-Wunused-matches), and fix a stray double space in its signature. - Dispatch: remove the now-dead Web.PathPieces import; fix an unbalanced TH name-quote in nestedDispatchCall's haddock that misrendered. - Class/Dispatch: narrow the open Network.HTTP.Types import to the two symbols actually used (status301, status307), matching master's style. - Routes/TH/Internal: drop unused RecordWildCards and ViewPatterns pragmas. Changed modules now build clean under -Wunused-imports; full suite green (tests + test-routes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the pre-merge review: factor out genuine duplication and sharpen a few names, with no behavioral change (tests + test-routes green, warning-clean under -Wall -Wunused-imports -Wname-shadowing). RenderRoute: - Hoist the verbatim-duplicated `mkPieces` and `isDynamic` helpers (one copy each in mkRenderRouteClauses and mkRenderRouteNestedClauses) to single module-level definitions. - Factor the duplicated child-deriving-context rule into `childDerivCxt`; both mkRouteConsOpts and mkRenderRouteNestedInstanceOpts now delegate. - Rename the focused-children generators mkRouteConsOpts'/mkRouteCon' to mkFocusedChildCons/mkFocusedChildCon and document why they are deliberately NOT merged with the in-module mkRouteCon: the latter must honour the focus gate + InlineCompat branch, while here every tree is already inside the focused target, so applying that gate would wrongly skip grandchild instances. Dispatch: - Rename route'/route -> thisRoute/fullRoute (and the subsite lambda to routeBuilder): thisRoute is this route's own constructor applied to its pieces; fullRoute wraps it in the enclosing parent constructors. ParseRoute: - Factor the repeated `_ -> Nothing` trailing clause into top-level `missingRouteClause`, and the repeated `applyTyArgs (ConT (mkName n)) origTyargs` into a local `targetTypeFor`. - Drop the thin (unexported) `generateParseRouteClausesInline` pass-through; its sole caller now uses `buildInlineParseClauses` directly, with the relevant haddock folded in. - Inline the single-use `recordName` into `recordNameIfNotInstance` (when not . member -> unless). Core/Internal/TH: - Factor the duplicated parseYesodName-then-fail into `parseYesodNameQ`. - Replace the recursive `findNested` local with a list comprehension. - Rename the `parseRoute` local to `parseRouteDec` (matches its dispatchDec/resourcesDec siblings; clears a name-shadow of the imported parseRoute). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses L0neGamer's review question ("can we assert these are the only
constructors of FocusNestR?"). The name-list reflection that previously
answered it (lift the Con names, compare at runtime) is replaced by a
total `describeFocus :: FocusNestR -> String` case, with the module
escalating -Werror=incomplete-patterns. This pins the generated set at
compile time and more cleanly:
* a missing constructor fails the pattern reference (name not in scope);
* an extra constructor makes the case non-exhaustive, which the
-Werror turns into a build failure.
Verified the guard actually bites: dropping the FocusShowR arm fails the
build with "Patterns of type 'FocusNestR' not matched: FocusShowR _".
Drops the now-unneeded lift/conName/SigD-ValD splice machinery and the
Language.Haskell.TH.Syntax (lift) import.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… tested
L0neGamer suggested three probe cases. The concrete-applied one
(instance Probe (HasInstInt Int)) is already covered. The other two —
instance Probe HasInst1 (unapplied arity-1) and instance Probe
(HasInst2 a) (partially-applied arity-2) — are not representable: Probe
is Type-kinded like the real nested-discovery classes, so a head of kind
Type -> Type is kind-rejected at its definition site ("Expected a type,
but ... has kind * -> *"). Verified by adding both to this module and
watching GHC reject them.
Generalize the existing unapplied-constructor note to name both
under-applied shapes explicitly, so the code answers the review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ests L0neGamer suggested probing instances written at under-applied heads (instance at a bare arity-1 constructor, and at a partially-applied arity-2 one). Those aren't representable against the Type-kinded Probe — GHC kind-rejects the instance head — so make them representable with a poly-kinded `class ProbePoly (a :: k)` and pin what the probe does: - PolyFull (instance ProbePoly (PolyFull a), fully applied) -> True - PolyUnapplied (instance ProbePoly PolyUnapplied) -> False - PolyPartial (instance ProbePoly (PolyPartial a)) -> False The False results are the point: nestedInstanceExists saturates the datatype to its own arity (a kind-Type head, PolyUnapplied a / PolyPartial a b) before querying isInstance, so an instance at the bare or partially-applied constructor is a *different* head and does not match. The probe queries at the fully-applied head specifically and isn't fooled by an under-applied instance. Verified empirically (the expectations were confirmed by the run, not assumed). Generalize the existing note to cross-reference ProbePoly, and merge the duplicate import. test-routes: 107 examples, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Direct isInstance queries at hand-built heads, documenting the matching boundary that justifies fullyApplyType saturating to the datatype's full arity: - ProbePoly (PolyPartial Int) [one arg short, kind Type->Type] -> True - ProbePoly (PolyPartial Int Int) [full, kind Type] -> False - ProbePoly (PolyUnapplied Int) [kind Type] -> False The partial instance ProbePoly (PolyPartial a) matches *any* one-arg application, so a probe that stopped one argument short would be spuriously fooled by it; saturating to the full arity (PolyPartial a b, kind Type) is what avoids that. test-routes: 110 examples, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per L0neGamer's review note. The Dispatch block round-tripped only "/"; add "/foo", which dispatches to FirstFooR's index handler (FooIndexR). test-routes: 111 examples, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per L0neGamer's review: once this PR merges, mkYesod/mkYesodData are the
canonical ways to use yesod, so a split fragment should model them.
Replace Nest.hs's three low-level focused builders
(mkRenderRouteInstanceOpts + mkRouteAttrsInstanceFor +
mkParseRouteInstanceFor) with the single bundled
mkYesodDataOpts (setFocusOnNestedRoute "NestR" defaultOpts)
"Hierarchy" hierarchyResources
which emits exactly the same focused render/attrs/parse instances (its
data half is [parseRouteDec, renderRouteDec, routeAttrsDec], honoring the
focus). This mirrors the existing Hierarchy.Nest3 fragment.
No coverage lost: the sibling fragments Admin, Nest2 and Nest2.NestInner
still drive the underlying per-instance builders directly, so the
route-level generators mkYesodDataOpts bundles stay exercised. Updated
the module haddock to say so. hierarchy spec: 111 examples, 0 failures,
warning-clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
OK I think all review comments are addressed. I'd like to merge tomorrow and release; if you have any blocking comments, feel free to leave 'em! |
L0neGamer
approved these changes
Jun 16, 2026
L0neGamer
left a comment
L0neGamer
left a comment
Contributor
There was a problem hiding this comment.
The changes seem good, although I haven't look at the latest stuff in as much detail as previous changes.
Likely good to go when CI is fixed!
The InstanceProbeSpec HasInstInt case probes the abstract head `HasInstInt a` against the concrete `instance Probe (HasInstInt Int)`. GHC 9.0+ reifyInstances returns unifiers (so it matches → True), but GHC 8.8/8.10 do not unify a bare type-variable query head against a concrete-argument instance (→ False), failing the lts-16/lts-18 CI jobs. This concrete-argument shape never arises in real codegen (nested discovery instances are always emitted at fully-abstract parameters, resolved identically on every supported GHC), so CPP-gate the expected value per GHC version to pin the observed reifyInstances divergence rather than a behaviour the codegen depends on. Update the HasInstInt doc comment to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
parsonsmatt
merged commit e7601d9
into
mattp/route-behavior-tests
22 of 26 checks passed
parsonsmatt
added a commit
that referenced
this pull request
Jun 16, 2026
* Pin existing routing behavior with regression tests
Extracted from #1887 (split route compilation). These tests assert
behavior that yesod-core already has on master:
- ParameterizedSubDispatchRuntime: end-to-end dispatch through a
parameterized subsite (associated types, ConstraintKinds, dynamic
parent pieces), including 404s and method mismatches
- MultiPieceNestedRuntime: nested parents whose prefix captures
dynamics (/shop/#Int/admin), forwarding parent+child captures
- ZeroPieceShadowRuntime: a zero-piece nested parent (/) shadows
later siblings - longstanding, previously unpinned behavior
- BangSeparatorRuntime: /!#Int dispatches like /#Int
- RuntimeHarness: shared WAI round-trip assertion helper
Running them against master first shows the feature branch preserves
all of this; #1887 will be rebased on top so its diff no longer
carries these files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Code generation and delegation to nested routes (#1887)
* when showing HandlerContent, include some information from the Content
* add PR link
* change version number
Co-authored-by: Matt Parsons <parsonsmatt@gmail.com>
* truncate lazily and decode as UTF-8
* respect content type better when generating snippets
* handle some rare encodings
* doc string and style
* remove unused imports
* adopt style recommendations from PR review
* rearrange some things to avoid a breaking change
* Allow disabling of resources generation
* merge with master
* ChangeLog
* fix build
* Add notes
* Add notes
* Check to see if the datatype exists before creating it
* Begin implementing RenderRoute separation
* well that is weird
* Test route datatype generation
* Import to avoid generation
* ok cool
* routeAttrs separation
* Suffix is unnecessary
* Implement skipping in MkDispatchSettings
* parseRoute working for static paths, need to account for dynamic paths
* oops
* oh there just aren't tests for route parsing
* Route parsing tests all pass
* Remove splices
* Fix routeAttrs bug
* A few more tests
* remove eval comment
* hmmm
* ok better design
* steps to take
* Handwrite instance
* OK, it breaks informatively now
* dispatch???
* tests for NestR dispatch
* refactor tests a bit
* Tests for nested with route fragments
* progress
* we so close
* initial attempt at parent dyns
* Works with mkYesod
* all tests pass
* ok it compiles
* Add more tests for nested dispatch
* TH backcompat
* no notes
* simplify
* changelog, todo, bumps
* cleanup pass
* better docs
* woof
* Version number bump
* notes, again
* nice some good progress
* tests pass
* Failing - need to know how to return Nothing in a yesodRunner context...
* parsing code is building, not fully complete
* route parsing appears to work
* ok it is not working
* ok it is not working
* Route parsing compiles
* Cool, now need to fix the ParseRouteNested to be direct recursion...
* add todo note
* route parsing is done and tested
* Relocate the dispatch instance so there's only one file to be concerned with
* RenderRouteNested: begin
* making some good progress
* Claude did alright here
* Fix 404 generation
* Cleaned up tests
* Add type annotation
* Properly pass type argument in YesodDispatchNested
* Fix parseroutenested with parameterized routes
* don't spuriously demand constraints
* Correctly apply parent type constructor
* Fix parent args and dyns
* OK, in a good place.
* Establish the pattern of nested route generation
* ok, let's see what Claude can do
* Code gen and delegation works:
* a few more tests
* Clean up warnings
* cleanup
* Remove Claude file
* hm. a thorn.
In order to call yesodDispatchNested, I need to provide a function `a ->
Route site`. This function should be programmatically generated, and
corresponds to the composed constructor. This requires that nested
dispatch knows about the full `Route app`, but that's not a huge deal.
* Add a class for promoting to parent route
* ah yes of course
* got some nice todos
* sick
* all tests are passin
* toWaiAppYre now uses toWaiAppYre'
* expose these boys so i can use 'em in hspec-yesod
* UrlToDispatch class
* UrlToDispatch ought to do what we need here
* Add missing module
* Expose constructing a YesodRunnerEnv
* expose urltodispatch
* Add instance for tuples
* there can be only one
* Fix handlers with different types on response bodies
* Ensure that you can access a nested route without a slash
* kinda hacky but it does make things somewhat nicer
* Support unitary routes transparently
* smarter detection for weird superclass instances
* Move generation of UrlToDispatch convenience instances
* ok maybe i do want to conditionally have the yesod thing sigh
* ok maybe here is the right spot
* ok maybe this does it right
* sigh
* Actually build children if they're not already made
* Fix parameterized subsites
* fix tests
* Fix isInstance calls to use fully-applied types for parameterized routes
When nested route types have type parameters (e.g. from parameterized
subsites), isInstance requires fully-applied types. Added fullyApplyType
helper that looks up type arity and applies fresh type variables. Fixed
all isInstance checks across Dispatch, RenderRoute, ParseRoute, and
RouteAttrs TH modules.
This fixes the "Expecting one more argument to 'RouteR'" error when
using mkYesodSubDispatch with parameterized subsites that have nested
routes.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add compile-only test for mkYesodSubDispatch with parameterized types
Tests that mkYesodSubDispatch compiles when the subsite has
constrained type parameters (e.g. ParamSubDispatchClass subsite master
=> ParamSubDispatch subsite). This covers the isInstance / fullyApplyType
fix from the previous commit.
Note: mkYesodSubDispatch + nested route delegation (`:` syntax) is not
yet supported for subsites because yesodDispatchNested expects
YesodRunnerEnv but subsite dispatch uses YesodSubRunnerEnv. The
ParameterizedSubData module already covers mkYesodSubData with nested
routes.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add YesodSubDispatchNested class and inline dispatch for subsite nested routes
When a subsite has nested routes (`:` syntax), mkYesodSubDispatch now
handles them via two mechanisms:
1. Inline dispatch (backward-compatible): When no nested dispatch instance
exists, mkDispatchClause's `go` for ResourceParent recursively generates
child dispatch clauses inline. Existing user code using mkYesodSubDispatch
works with nested routes without changes.
2. Class-based delegation: New YesodSubDispatchNested class (parallel to
YesodDispatchNested) enables module separation for subsite nested routes.
mkYesodSubDispatchInstance generates both YesodSubDispatch and
YesodSubDispatchNested instances in one splice.
Also incorporates l0neGamer's PR #1910 test case with associated types,
ConstraintKinds, and dynamic pieces in nested route parents.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix CI failures on older GHC versions
- Replace [t| ParentSite $(pure x) |] TH splices with manual type
construction (ConT ''ParentSite `AppT` x) to avoid $ being parsed
as a type operator on GHC < 9.4 (lts-16, lts-18)
- Add data-default to tests suite build-depends since hs-source-dirs
includes src/ and Dispatch.hs conditionally imports Data.Default
when wai-extra < 3.1.14 (lts-20)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix foldMap over Q on GHC 8.8/8.10
Replace foldMap with mapM + mconcat since Monoid (Q (a, b)) isn't
available on older GHCs that lack Monoid a => Monoid (Q a).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix GHC 8.8/8.10 compatibility in ParseRoute and RenderRouteSpec
- Replace Quote-based polymorphism with explicit liftQ in
generateParseRouteClause (Quote class doesn't exist pre-GHC 9.0)
- Add CPP guards for ConP arity change (template-haskell 2.18/GHC 9.4)
and TupE Maybe wrapping (template-haskell 2.16/GHC 8.10) in tests
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix pathInfo not in scope in generated subsite dispatch code
Use TH quoted name 'W.pathInfo instead of mkName "pathInfo" in
record update expression within genNestedDispatchClauses. The
mkName version creates an unqualified name that requires
Network.Wai to be imported at the splice site.
Fixes error reported by l0negamer: "Not in scope: record field
'pathInfo'" when using mkYesodDispatchOpts with nested routes.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Use TH quoted names and newName for better hygiene
- Replace mkName "renderRoute" with 'renderRoute in RenderRoute.hs
for consistency with 'renderRouteNested usage
- Replace mkName lambda-bound variables (sroute, p, r) with newName
in Dispatch.hs to avoid potential name capture
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add mkLambda helper for hygienic TH lambda generation
Introduces mkLambda in Internal.hs that generates a single-argument
lambda with a fresh name, replacing manual newName + LamE patterns
across Dispatch.hs for better name hygiene and less boilerplate.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add attributes directly to resource parent (#1911)
* Initial commit
* Add basic tests for parent attr inclusion
* Minimize language extensions
* Add since annotation based on directions
* Add Lift instance for Set from th-lift-instances
* Add th-lift-instances
* Add th-lift-instances to test targets
* Use record to represent parent details over tuple
* Add changelog entry
* Update ChangeLog
* Make subsite/nested-route discovery backwards-compatible (opt-in)
The nested-route-discovery rewrite emitted its machinery by default, which
broke parameterized subsites: nested subroute datatypes silently gained the
parent's type parameter (e.g. `data NestedR` became `data NestedR subsite`),
breaking any downstream type signature mentioning the subroute.
Gate the whole nested layer behind a single predicate, `useNestedDiscovery`,
defined in RenderRoute and shared by ParseRoute and Dispatch:
useNestedDiscovery opts tyargs =
null tyargs || roParameterizedSubroute opts || isJust (roFocusOnNestedRoute opts)
- Monomorphic sites (null tyargs) keep nested discovery on, preserving the
working cross-module splitting (NestedDispatch).
- Parameterized sites/subsites that have NOT opted in fall back to the
historical inlined output: unparameterized subroutes, inline renderRoute /
parseRoute, no *Nested / ToParentRoute instances. This is byte-for-byte
master, verified by porting the parameterized-subsite tests onto master.
- Opting in via setParameterizedSubroute (e.g. mkYesodSubDataOpts) restores
parameterized subroutes and the full machinery.
Also restore subsite route-splitting: mkDispatchClause always runs the
isInstance check, so a subsite's dispatch delegates to a YesodSubDispatchNested
instance compiled in another module (and inlines when none exists, staying
backwards compatible). Suppression of YesodDispatchNested generation for
opted-out parameterized sites moves to the mkDispatchInstance call site, so the
redundant mdsUseNestedDiscovery field is removed.
Tests:
- _nestedRArityGuard: default subsite subroutes stay kind Type (oracle:
compiles+passes on master).
- _editorRParamGuard / ParamSubsiteParameterized: opt-in yields parameterized
subroutes (kind Type -> Type).
- SplitSubsite: end-to-end cross-module subsite route splitting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Refine nested-route discovery: TyArgs ADT, pure parse codegen, tests
Follow-up quality/test work on top of the backwards-compatible nested-route
discovery (c331b994). Behavior is unchanged; both suites green (tests 266,
test-routes 76).
TyArgs ADT
Replace the bare `[(Type, Name)]` + `null` checks threaded through the
generators with `data TyArgs = NoTyArgs | SomeTyArgs (NonEmpty (Type, Name))`
(in Yesod.Routes.TH.Types) plus accessor helpers (toTyArgs, tyArgsTypes,
tyArgsBinders, tyArgsArity, hasTyArgs, applyTyArgs). applyTyArgs subsumes the
recurring `foldl' AppT con (fst <$> tyargs)` folds; the `case tyargs of
(_:_)/[]` matches become SomeTyArgs{}/NoTyArgs. Converted every signature and
consumption site across RenderRoute, ParseRoute, Dispatch; entry points in
Core/Internal/TH build it via toTyArgs.
Pure inline parseRoute codegen
Extract the effect-free core of generateParseRouteClausesInline into a pure
buildInlineParseClauses (State Int fresh-name supply + direct AST instead of
newName + quotation brackets). The Q function is now a thin shell. Unit test
Route.InlineParseClausesSpec asserts on the [Clause] output with no splice /
no runQ.
discoveryMode classifier
Named DiscoveryMode (InlineCompat | NestedDiscovery) sum + discoveryMode
classifier replacing the useNestedDiscovery predicate; decision bound once and
consumed via case at each site. Unit-tested in Route.DiscoveryModeSpec.
mkYesodSubDispatchInstance arity guard
Pure checkNestedSubArity gives an actionable error (instead of a cryptic kind
error) when a parameterized subsite is paired with an unparameterized nested
datatype; factored typeArity out of fullyApplyType. Unit-tested in
Route.SubDispatchAritySpec.
Runtime coverage
Add ParameterizedSubDispatchRuntime (+Data) and ParamSubDispatchInstanceRuntime
exercising the opt-out inline path and mkYesodSubDispatchInstance end to end
(round-trip + WAI dispatch).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add runtime coverage for previously-untested route combinations
Fill three gaps in the route-codegen test matrix, all exercised end to end
(WAI dispatch + parseRoute/renderRoute round-trips). tests suite 266 -> 291.
- Parameterized subsite, nested routes split across modules
(ParamSplitSubNested + ParamSplitSubRuntime). The headline gap: split was
monomorphic-only (SplitSub) and parameterized nested-discovery was
single-module-only (ParamSubDispatchInstanceRuntime); their intersection had
no coverage. The nested PNestedR handlers live ONLY in the split module and
its YesodSubDispatchNested instance is generated there with the subsite's
type argument threaded through mkNestedSubDispatchInstance as SomeTyArgs, so
the parent mkYesodSubDispatch splice compiles only by delegating to it
(inlining would reference handlers not in scope) -- the build is the
delegation proof. Exercises the SomeTyArgs/applyTyArgs dispatch branches.
- Top-level parameterized site, opted into nested discovery
(ParamTopLevelRuntime). Existing coverage (ParameterizedSite.SubRoute) fires
one request per route with no round-trip or error cases; this adds full
round-trips plus 404/405 on the same opt-in top-level path.
- Fallthrough on a parameterized site (ParamFallthroughRuntime). The
monomorphic fallthrough test covered only an unparameterized site; this is
the parameterized counterpart (setNestedRouteFallthrough + opted-in nested
discovery) with several same-prefix parents falling through to one another.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Round-2 cleanup: finish dedup, tighten arity guard, harden tests
De-duplication (each extraction previously stopped one site short):
- handlePieceM/handlePiecesM: abstract the piece->pattern logic over the
fresh-name supply (Applicative m => String -> m Name), replacing the three
near-identical copies in ParseRoute (Q and State) and Dispatch.
- delegatingBody: hoist to module scope so the mkRenderRouteNestedClauses
subsite arm shares it instead of carrying a byte-identical copy.
- appliedRouteType :: Name -> TyArgs -> Q Type: single helper for the
apply-tyargs-or-reify-and-saturate block (5 call sites).
- Reuse typeArity in Core/Internal/TH instead of hand-rolling reify+arity.
Correctness:
- checkNestedSubArity now requires subArgs == nestedArity. Dispatch applies
exactly the subsite's type args to the nested datatype, so both over- and
under-application leave the instance head ill-kinded; only equality is sound.
Flip the over-arity test case to expect Left.
Cleanup:
- Single lookupTypeName in the nested-instance loop, reused for the existence
probe and the arity check; skip the arity check (and its misleading
"0 type parameter(s)" message) when the datatype name doesn't resolve.
- Refresh the stale discoveryMode haddock (hasTyArgs/NoTyArgs, not "not . null").
Tests:
- InlineParseClausesSpec: assert structurally that the parent's dynamic binder
is threaded into the reconstructed body (AST walk), not just present in pprint.
- ParameterizedSubDispatchRuntime: add malformed-dynamic 404 cases.
Verified: yesod-core builds; 294 tests + 77 test-routes, 0 failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Release as 1.7.0.0: ChangeLog + reconcile @since tags
Nested-route discovery is a breaking change (the route TH entry points now
take TyArgs instead of bare lists, and useNestedDiscovery was replaced by
DiscoveryMode/discoveryMode), so this cuts a major version.
- Bump yesod-core to 1.7.0.0.
- Retag the new exports' @since from the placeholder 1.6.30.0 to 1.7.0.0.
- Promote the ChangeLog UNRELEASED section to 1.7.0.0 and document the
subsite nested-route discovery, the DiscoveryMode/discoveryMode classifier
(replacing useNestedDiscovery), and the TyArgs API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Round-3 cleanup: finish dedup sweep, harden fallthrough + parse tests
De-duplication (three more copies of the round-2 patterns):
- handlePieceM now yields the bound Name; handlePiecesNames/handlePiecesM
project it to Names/Exps. Dispatch's genPiecePats (the third piece-handling
copy) is deleted and its two callers use handlePiecesNames.
- appliedRouteTypeNamed :: String -> TyArgs -> Q Type folds the byte-identical
lookupTypeName+fallback wrapper that surrounded appliedRouteType at two sites.
- mkPathPat hoisted to Internal.hs; the where-local copy in mkDispatchClause,
the top-level copy in ParseRoute, and the consPat helper all use it.
- The one remaining delegation probe still calling fullyApplyType directly now
uses appliedRouteType, matching every other probe (behaviour-preserving:
NoTyArgs is exactly fullyApplyType).
Tests:
- ParamNoFallthroughRuntime: the disabled-fallthrough counterpart — same
overlapping /foo parents without setNestedRouteFallthrough, asserting a 404
where the enabled version falls through. Pins roNestedRouteFallthrough so an
always-on regression is caught.
- InlineParseClausesSpec: the threading assertion is now a single non-vacuous
check (`map (elem used) dynVars == [True]`), and clauseBodyExp is total.
ChangeLog: note the checkNestedSubArity == tightening and the unresolved-name
skip in the arity guard.
Verified: yesod-core builds; 297 tests + 77 test-routes, 0 failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-A: fix #1 ill-kinded delegation probe + regression test
The go-delegation probe in mkDispatchClause saturated the in-scope child
route datatype with the *site's* TyArgs. In the InlineCompat path the child
is generated at kind Type regardless of site arity, so for a parameterized
site this built an ill-kinded head (e.g. SubParentR a over a kind-Type
SubParentR); isInstance then *threw* a kind error and aborted the splice
rather than returning False. Saturate by the child's own reified arity
(fullyApplyType) instead, restoring master's behavior.
Also rewrites the stale/wrong comment (#13) to reference function names.
Adds ParamDefaultSplitData/ParamDefaultSplitRuntime: parameterized site +
default opts + nested parent + split mkYesodData/mkYesodDispatch. Verified
non-vacuous (reproduces "Expected kind 'k0 -> *', but 'SubParentDR' has kind
'*'" without the fix).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-B: RouteCon — resolve route names once, single nested-instance probe
Introduces RouteCon{rcName,rcResolved} + resolveRouteCon in
Yesod.Routes.TH.Internal: a route name is resolved once at the boundary and
the answer carried in a value, rather than re-running lookupTypeName and
independently re-deciding what an unresolved name means at each use.
- nestedInstanceExists :: Name -> RouteCon -> Q Bool collapses the seven
copy-pasted lookupTypeName+isInstance delegation probes (Dispatch go /
YesodDispatchNested / YesodSubDispatchNested, RenderRoute x2, ParseRoute,
RouteAttrs, Core.Internal.TH). It always saturates by the datatype's own
reified arity (fullyApplyType), never the site's TyArgs — the divergence
that caused #1 — so the probe can never build an ill-kinded head and abort
the splice. This fixes #1/#2 by construction.
- appliedRouteTypeCon :: RouteCon -> TyArgs -> Q Type unifies the former
contradictory appliedRouteType (Name; reify-or-throw) and
appliedRouteTypeNamed (String; mkName-fabricate) policies in one place;
appliedRouteTypeNamed is now a thin resolveRouteCon wrapper.
- #2 dissolved: the subsite probe now precedes checkNestedSubArity without
preempting it, since the probe no longer throws on arity mismatch.
Both suites green (307 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-C (part): dedup route construction, RenderRouteNested body, test harness
- #11 applyConPieces :: String -> [Exp] -> Exp in Yesod.Routes.TH.Internal,
the single spelling of the foldl' AppE (ConE (mkName name)) idiom; replaces
the ~13 copies across Dispatch (incl. the two divergent point-free/lambda
spellings) and ParseRoute (which no longer needs Data.List).
- #10 renderRouteNestedBody folds the piecesAndNames -> parentDynT ->
parentNames -> mkRenderRouteNestedClauses kernel that was triplicated across
the three RenderRouteNested instance sites; only the instance head/context
wrappers remain. The three error "empty name????" partials collapse to one
fail with an attributable message.
- #12 YesodCoreTest.RuntimeHarness.assertRequest is the one shared WAI
round-trip; the seven runtime specs' duplicated testRequestIO (in two
disagreeing shouldBe/assertStatus styles) now each delegate in one line.
Both suites green (307 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-C (#8): fold the two nested-dispatch instance twins into one
mkNestedDispatchInstance (top-level) and mkNestedSubDispatchInstance (subsite)
were ~80-line near-duplicates differing only by the two Names that
NestedDispatchConfig already carries (ndcDispatchFn/ndcDispatchClass) plus the
top-level-only master site. Both are now one-line wrappers over a shared
mkNestedDispatchInstanceWith driven by the config, with a Maybe Type master:
Just master ⇒ top-level (also emits UrlToDispatch/RedirectUrl, now extracted
to mkUrlToDispatchRedirectInstances); Nothing ⇒ subsite.
Both suites green (307 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-D (#5, #6): PathTail ADT + typed checkNestedSubArity
#5: mkPathPat's tail was an arbitrary Pat — a caller could pass any pattern
and silently match the wrong number of segments. Replace with a closed
PathTail = EndExact | EndRest Name | EndWild | EndMulti Name (+ pathTailPat).
All ~12 dispatch/parse call sites and handleDispatch now speak PathTail.
#6: checkNestedSubArity took two swappable Strings and two swappable Ints and
returned Either String (). Now newtype SubsiteName/RouteName/SubsiteArity/
RouteArity make every argument non-swappable, it returns Either ArityMismatch
Arity (the proven arity, not discarded ()), and the message is rendered
separately via arityMismatchMessage so the test asserts on structure.
Both suites green (307 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-D (#14): error -> fail in Q generators; drop dead helper
error inside a TH splice aborts with no source attribution; fail routes
through Q's error reporting and names the splice site. Converted the
defensive generator failures:
- RenderRoute.hs mkPieces/mkParentPieces: lifted from pure [Exp] to Q [Exp]
so the "dynamic piece without a bound variable" guard can fail with a
descriptive message instead of error "mkPieces 120".
- Core/Internal/TH.hs: the two appCxt builders now traverse (Q) instead of
fmap (pure), so the empty-context guard fails with a real message.
- Dispatch.hs: deleted mkUrlToDispatchNestedInstance entirely — it was an
unexported, unfinished stub whose body was error "" (truly dead), so it
is removed rather than converted.
Both suites green (307 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-D (#15): unify ParseRoute.hs on a single AST dialect
generateParseRouteClause built its tuple patterns and fmap/parseRoute(Nested)
bodies through liftQ [p| ... |] / [e| ... |] quotation brackets, while its
pure sibling buildInlineParseClauses built the identical shapes with plain
constructors (TupP / mkTupE / VarE 'fmap). A change to the parsed tuple shape
had to be mirrored in both dialects.
Converted generateParseRouteClause to the plain-constructor dialect:
TupP [pathPat, VarP qp] for the (path, query) pattern, mkTupE for the rest
tuple, and VarE 'fmap / 'parseRoute / 'parseRouteNested for the bodies. No
behavioural change — same patterns, same expressions. Dropped the now-unused
Web.PathPieces import.
Both suites green (307 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* PR-D (#16): commit to 404 in the delegated no-fallthrough ResourceParent arm
genNestedDispatchClauses' ResourceParent arm had asymmetric fallthrough
handling. With fallthrough enabled it pattern-guards on Just (so a child's
Nothing falls through to siblings); with fallthrough disabled it returned the
child's raw `nestedCall :: Maybe _` as the Match body. The inline dispatch path
(mkDispatchClause's go) instead converts a no-fallthrough child's Nothing into
a committed 404. The delegated path now mirrors that:
Just (fromMaybe (runner (void notFound) yre Nothing req) nestedCall)
so a no-fallthrough parent commits to its subtree regardless of what the
calling context does — matching the inline shape. The fallthrough (guard) arm
is untouched.
Within a single splice (uniform fallthrough flag) the old Nothing already
propagated up to the top-level go's Nothing->404 conversion, so observable
behaviour is unchanged there — which is why the full suite stays green. The
fix matters for the mixed case (an inner parent's nested instance generated
with fallthrough disabled, dispatched to by an outer fallthrough caller),
where the raw Nothing would otherwise leak through to sibling routes.
Added ParamNestedNoFallthroughRuntime: a three-level (grandparent -> parent ->
leaf) no-fallthrough site under nested discovery. Unlike the two-level
ParamNoFallthroughRuntime (whose parent only dispatches leaf children), this
drives the delegated ResourceParent arm and locks in the committed-404
behaviour for a deep miss.
Both suites green (311 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Housekeeping: delete orphaned, bitrotted bench/th.hs
bench/th.hs (and its private helper bench/THHelper.hs) was never a cabal
target — the only benchmark stanza is `widgets` (widget.hs) — so it has not
compiled in a long time and bitrotted past repair:
- imports Yesod.Routes.TH.Simple / mkSimpleDispatchClause, which no longer
exist anywhere in the tree (nothing to migrate to);
- calls mkRouteConsOpts defaultOpts [] [] ... and mkDispatchClause settings
resources with their pre-TyArgs arities (mkRouteConsOpts now takes
RouteOpts -> Cxt -> TyArgs -> Type -> ..., mkDispatchClause takes six args);
- THHelper.hs builds MkDispatchSettings positionally, but that record has
since gained several fields.
There is no current API to migrate the "simple vs generated dispatch" micro-
benchmark onto, so the unit is deleted. The unrelated non-th.hs / pong.hs
orphans are left as-is (not part of this benchmark unit).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Mark round-4 review TODO as fully resolved
All findings (#1-#16 + bench/th.hs housekeeping) implemented across PR-A..PR-D
and the housekeeping commit; #7 needed no action. Both suites green (311 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* R10 + T5: delegatees honor their Nothing-on-miss contract
yesodDispatchNested / yesodSubDispatchNested are documented to return Nothing
on a miss so the *caller* decides what a miss means. But a nested instance
generated with the default fallthrough = False baked a 404 into its own
terminal fallback and returned Just — so when routes are split across modules,
a child compiled with the default flag silently overrode a fallthrough-wanting
parent compiled elsewhere.
R10 makes the contract honest:
- genNestedDispatchClauses' terminal-miss clause is now unconditionally Nothing
(was flag-conditional Just-404).
- mkClause404's NestedDispatch branch is now unconditionally Nothing; only the
TopLevelDispatch terminal authority commits a final miss to 404.
The commit semantics of fallthrough = False already live at the delegating
clauses — the inline parent body in mkDispatchClause (Nothing -> Just 404 case)
and the genNestedDispatchClauses ResourceParent arm (Just (fromMaybe notFound
nestedCall), from the earlier #16 change) — and at the terminal authorities
(top-level toWaiAppYre' / inline clause404, and the subsite ComposedEnv path).
Net rule, stated as a feature not a caveat: the module containing a parent
route decides what happens when that parent's subtree misses. No API change;
the flag is kept. Safe to do outright — the nested-fallthrough feature is
branch-only / unreleased.
T5 — new FallthroughMatrix cross-module cell: a top module with
fallthrough = True delegating FirstFooR to a split instance generated with the
default False, plus an inline SecondFooR sibling sharing the /foo prefix.
GET /foo/other must fall through FirstFooR (no "other" child) to SecondFooR's
OtherR. This asserted 404 before R10 (bug reproduced) and 200 after.
Subsite ComposedEnv terminal ownership is already covered: ParamSplitSubRuntime
/ ParamSubDispatchInstanceRuntime / ParameterizedSubDispatchRuntime each assert
404 on an unknown nested suffix and stay green — confirming the subsite terminal
authority still converts the now-Nothing nested miss into a 404. Uniform-flag
behavior is unchanged (ParamFallthrough 200, ParamNoFallthrough /
ParamNestedNoFallthrough 404 all still hold).
Both suites green (314 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* R1+R2: delete dead mkUrlToDispatchInstances + stray repo-root notes
R1: mkUrlToDispatchInstances (Dispatch.hs) was unexported and unreferenced
anywhere in the repo (only a stale ctags entry pointed at it). It generated
UrlToDispatch/RedirectUrl instances for a setUrl-from-hspec-yesod path that is
no longer wired up. Removed, along with its private applyTypeVariables helper.
R2: removed the stray repo-root scratch files notes.md,
notes-seturl-callstack.md, and TODO.md (the round-4 review, now folded into the
tracked plan). bench/th.hs was already deleted earlier.
Both suites green (314 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* R3+R4: dedup RouteAttrs helpers + tyvarbndr CPP block
R3: RouteAttrs.hs defined its own local instanceD and mkConP (the latter with
a template-haskell-version CPP shim). Both already exist as the shared
Internal.instanceD / conPCompat — switch to those and drop the local copies
(and the now-unneeded CPP pragma).
R4: the PlainTV/TyVarBndr CPP shim appeared three times in RenderRoute.hs
(once per generated-subroute-datatype site), each binding a local `tyvarbndr`.
Collapsed into one top-level plainTVCompat helper (its result type is CPP-
guarded too, since the TyVarBndr flag type varies by version); the three sites
now just `fmap plainTVCompat (tyArgsBinders tyargs)`. Binder shape unchanged.
Both suites green (314 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* T3: runtime coverage for a multi-piece nested parent
Add MultiPieceNestedRuntime: a nested ResourceParent whose own path pieces are
>1 and include a dynamic capture (/shop/#Int/admin), with children at / and
/users/#Text. Asserts the parent index forwards the captured #Int, a child
forwards both the parent #Int and child #Text, a bad suffix 404s, and a
malformed parent dynamic (#Int given a non-integer) 404s. Exercises
handlePiecesNames / mkPathPat / the parentArgsExp tuple build for multi-piece
parents — previously only single-piece nested parents had direct runtime tests.
The plan's other coverage items are already satisfied by existing tests: T2
(bare-mkName arity fallback) by SplitSubsite's monomorphic mkYesodSubDispatch
nested dispatch, T4 (ComposedEnv runtime) by ParamSplitSubRuntime, T6 (arity
guard logic) by SubDispatchAritySpec, T8 (discoveryMode truth table) by
DiscoveryModeSpec. The remaining gaps (T1 dump-splices goldens, T7 compile-fail
harness, and the end-to-end splice-fail half of T6) were de-scoped.
Both suites green (318 + 77).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* R8+R9: ChangeLog + Haddock for nested-route fallthrough & parameterized subsites
R8: document, in the unreleased 1.7.0.0 ChangeLog section, the nested-route
fallthrough ownership rule (the module containing a parent route decides what a
subtree miss does; nested instances always return Nothing on miss) and the
parameterized-default split-dispatch kind-error fix.
R9: Haddock on setParameterizedSubroute and mkYesodSubDispatchInstance spelling
out the parameterized-subsite requirements — the subsite -> master functional
dependency and the UndecidableInstances / FlexibleContexts / FlexibleInstances /
MultiParamTypeClasses / TypeFamilies extensions the generated instances need,
plus a pointer to the nested-datatype arity rule.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address nested-route-discovery review findings (PR #1887)
Fix latent split-module codegen bugs, bump sibling version bounds, and
clean up the public API surface, per code review.
- Dispatch: handle multi-piece leaves in the nested-dispatch generator
(EndMulti + append the captured value) so split-out / subsite fragments
ending in *Texts/+Texts dispatch correctly; add InnerR runtime coverage.
- RouteAttrs: apply the accumulated `front` wrapper in the nested-instance
clause so a parent-with-instance under an inlined ancestor produces a
well-typed pattern.
- RenderRoute: saturate the child datatype in the ParentSite/ParentArgs
type-family heads instead of emitting a wildcard.
- Bump `yesod-core < 1.7` -> `< 1.8` across the 11 sibling packages.
- Correct stale `@since 1.6.28.0` -> 1.7.0.0 on the new exports; add an
@since to ToParentRoute.
- Re-add mkRenderRouteClauses & friends to the Yesod.Routes.TH umbrella;
drop the duplicate setCreateResources export.
- Delete the empty NestedDispatchFallthrough stub module.
tests: 324 examples, 0 failures (yesod-core, local resolver)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Resolve all 21 nested-route-discovery review findings
Backwards-compat: RedirectUrl overlap hazard note; export route-class
types without unqualified method names; drop the MkRouteOpts constructor
export (local record-match helper instead); document the full breaking
TH-API set + findOverlapNames narrowing in the ChangeLog.
Correctness: thread RouteOpts into subsite nested dispatch via
mkYesodSubDispatchInstanceOpts so fallthrough is reachable; generate
RouteAttrsNested for monomorphic single-module sites; run
checkNestedSubArity in the nested recursion (deep-arity guard);
fix/remove the dead shouldBeTH; doc + regression test for the
zero-piece-parent sibling-shadow; correct the base YesodDispatchNested
404 haddock.
Test coverage: nested-dispatch miss / deep-404 entry-point tests;
!-separator BackwardsR WAI-dispatch test.
Cleanup: delete dead focus-mode machinery (NestedRouteSettings/
DispatchPhase/determinePhase) and routeConArity; collapse the Arity
newtype; finish the dedup sweep — share parseYesodName/appCxt, extract
parentArgsExpr/Pat/Type, thread [Name] instead of decoding a Pat, and
de-fork the leaf/parent constructor builders (leafRouteCon/
parentRouteCon) shared by both RenderRoute generators; route the test
suites through RuntimeHarness (assertGet/assertRequestFor/
assertRequestRaw); Parse.hs uses fail not error for malformed-route
diagnostics in Q; resolve the tests-component double-vision; strip
leftover -ddump-splices pragmas; eliminate the O(nodes x depth)
findNestedRoute re-walk by threading the resolved subtree through the
nested-dispatch recursion.
Library + both test suites green (tests 337/0, test-routes 77/0).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Remove TODO.md review-tracking doc
All 21 findings resolved in da1205b7.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Resolve lingering nested-route-discovery review items
Correctness / tests:
- parseType/dropBracket: add MonadFail variants (parseTypeM/dropBracketM)
and derive Foldable/Traversable for the route AST types, so the TH
splice sites validate-and-convert route types in Q (parseResourceTypes)
— a malformed type or unclosed #{...} now fails as an attributed
compile error instead of a raw error thrown when the tree is forced.
The pure parseType/dropBracket are retained for known-good callers.
- Add Route.DeepAritySpec: drives the reify/typeArity path that the
nested-dispatch recursion uses (assertNestedSubArity under qRecover on
real datatypes), confirming a deep arity mismatch is caught by the
actionable guard rather than a cryptic kind error.
- Add YesodCoreTest.SubsiteFallthrough: a subsite with a nested parent
reaches a sibling after a nested miss only when generated with
setNestedRouteFallthrough True (200), and 404s with it off.
- arityMismatchMessage: take an ArityCallSite so the message names the
right entry point (mkYesodSubDispatchInstance vs mkYesod) and term
(subsite vs site) for the actual caller; a top-level site reached
through the recursion is no longer mislabeled a "subsite".
Release notes (ChangeLog):
- Document the new `encoding` dependency (absent from lts-22.x; needs an
extra-dep) and the coordinated companion-package republish required to
build against yesod-core 1.7 outside the monorepo.
Library + both test suites green (tests 341/0, test-routes 82/0).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Docs: add missing @since tags and fix Haddock nits
Documentation-only pass over the nested-route-discovery surface:
* Add `@since 1.7.0.0` to newly-introduced exported terms that lacked it:
`UrlToDispatch`/`urlToDispatch`, `mkYesodRunnerEnv`, `mkNestedDispatchInstance`,
`parseYesodName`, `mkParseRouteInstanceOpts`, `buildInlineParseClauses`,
`mkRenderRouteNestedClauses`, `ParentDetails`, and `WithParentArgs`. Where a
term had no Haddock at all, add a short description as well.
* Restore `mkYesodGeneralOpts`'s `@since 1.6.25.0`: inserting `parseResourceTypes`
between the tag and the function had detached it.
* Fix Haddock nits: "it's" -> "its" and backtick -> identifier-link markup in
`Yesod.Routes.Class`, empty `parseRouteNested` doc filled in, double-space after
`-- |` in `RouteAttrs`, and normalize the PR #1911 ChangeLog link.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Bump companion packages for yesod-core 1.7 release
Patch-bump the companion packages and add a "Support `yesod-core` 1.7"
ChangeLog entry to each, so they can build against and be published
alongside yesod-core 1.7.0.0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* reader and freshname consolidation
* Resolve max-effort review findings and rewrite the 1.7.0.0 changelog
Code fixes from a 12-issue sequential review-fix pass:
- Expose the documented nested-route API: ToParentRoute module exposed;
ParseRouteNested/RouteAttrsNested re-exported from Yesod.Core;
mkNestedSubDispatchInstance, TyArgs, parseType, dropBracket re-exported
from Yesod.Core.Dispatch. Split-recipe fixtures import the public paths.
- Thread setNestedRouteFallthrough into the subsite dispatch body via new
mkYesodSubDispatchWith (opts-less mkYesodSubDispatch keeps its 1.6 type).
- Deduplicate the RenderRoute focused/inline generators behind shared
builders, fixing two divergences (spurious Eq/Show/Read contexts on the
focused RenderRouteNested instance; mkRouteConsOpts dropping every leaf
constructor in focus mode).
- Fix parameterized + setFocusOnNestedRoute: apply TyArgs to the focused
RouteAttrsNested head; include reified-arity fill vars in TyArgs so a
parameterized foundation without explicit args is not misclassified
(top-level and subsite entry points).
- Restore 1.6 commit-on-parent-prefix semantics in InlineCompat parseRoute
so parseRoute agrees with dispatch under overlap-ignored routes.
- Fail loudly (shared findNestedRoute) on a missing focus target instead of
generating silent always-Nothing/always-mempty instances.
- Stop generating each nested subtree's dispatch twice: flat dispatch now
delegates to same-splice nested instances (mdsNestedDelegateInline), on
both the top-level and subsite paths.
- Move the behavioral tests suite onto the packaged library so hidden-module
regressions can no longer pass CI; internal codegen specs stay in
test-routes.
- New coverage: WithParentArgs/UrlToDispatch/RedirectUrl cluster, fallthrough
terminal 405/404 semantics, parameterized+focused fixtures, subsite
fallthrough opts, missing-focus-target failures, focus-mode leaf cons.
- Style sweep: drop mkDispatchClause's dead preDyns/parentCons params and fix
its haddock, remove write-only clause404 and unused _parentDepth, extract
resolveFoundation, correct the OVERLAPPABLE comment and the multipiece
diagnostic, add missing @since tags, delete dead test scaffolding and a
stray empty TypedContent.hs.
ChangeLog: rewrite the 1.7.0.0 section to be concise and user-facing
(Highlights/Improvements/Breaking changes/Notes); drop bullets that
documented fixes to bugs that never shipped in a release.
Test suites: tests 374 examples, test-routes 96 examples, 0 failures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address l0negamer review: Quote m, NestedTarget, String-typed focus setter, docs
Review round for PR #1887:
- Replace the FreshName class / State Int name supply with th-compat's
Quote m constraint; pure codegen tests run via test/Route/PureQ.hs.
- Replace the mdsNestedDispatchClass/Fn Name fields with a NestedTarget
enum (TopLevelNested | SubsiteNested), dissolving NestedDispatchConfig
and SubsiteEnvMode into derivations.
- setFocusOnNestedRoute now takes String directly, paired with
unsetFocusOnNestedRoute (no Maybe in the public API).
- Reattach floating haddocks, document RouteOpts defaults on defaultOpts
and each setter, drop -Wredundant-constraints.
- Document isInstance behavior with abstract type parameters and pin it
with test/Route/InstanceProbeSpec.hs; add purpose headers to test
fixtures.
- Rewrite the 1.7.0.0 changelog as flat per-PR bullets.
- Add docs/split-route-compilation.md: recipes for splitting top-level
and subsite routes, shared RouteOpts + fallthrough guidance, linking
to nested routes, troubleshooting.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fix CI on lts-16/lts-18: th-compat extra-dep, unambiguous newName
Two old-GHC breaks from the Quote m refactor:
- lts-16/lts-18 snapshots predate th-compat's entry into Stackage, so
the CI legs reusing stack-lts-22.yaml with an older --resolver failed
build-plan construction. Add th-compat-0.1.7 as an extra-dep.
- With template-haskell < 2.17, Language.Haskell.TH.Syntax exports a
monomorphic newName distinct from the th-compat Quote class method,
making every newName call site ambiguous. Hide it from the Syntax
import in Yesod.Routes.TH.Internal and ParseRoute; on TH >= 2.17 the
two names are the same entity, so nothing changes there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Keep ResolvedFoundation packed; project with accessors at use sites
Both resolveFoundation callers immediately destructured the record into
short local names (site, res, vns). Bind the record as `foundation` and
use rfSite/rfResources/rfFillVars/rfBoundNames at each use instead, so
the reads say what they are.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Replace mkYesodSubDispatchWithDelegate's Bool with SameSpliceNestedInstances
Boolean blindness: the Bool (and the mdsNestedDelegateInline field behind
it) encoded whether the caller emits the per-parent nested-dispatch
instances in the same splice. Name that decision as a two-constructor
type — GeneratesNestedInstances | NoSameSpliceNestedInstances — and use
it for both the MkDispatchSettings field and the entry point's argument.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Use quotation brackets over manual AST construction in new TH codegen
Sweep of the branch's new dispatch/render codegen for places where
manual AppE/RecConE/SigE chains could be quotes with splices for the
dynamic parts:
- nestedDispatchCall: quoted call, Proxy via type splice, and the
child wrapper as a composeE fold instead of a lambda.
- genNestedDispatchClauses: leaf body, YesodSubRunnerEnv construction
(both NestedTarget arms, now real record syntax), parent delegation
call, the 404-commit arm, handler method cases, and the path-drop
scrutinee.
- mkDispatchClause: the subsite dispatch expression and both 404
bodies (mkClause404, parentBody).
- RenderRoute: the two renderRouteNested delegation bodies; shared
consE helper replaces per-site [|(:)|] + local cons closures.
Left programmatic: anything driven by dynamic argument lists (piece
folds, constructor application, delegatingBody's tuple lambda) and
everything in the Quote m-polymorphic modules, where brackets are
Q-monomorphic on template-haskell < 2.17 and would break GHC 8.8.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* use quotes
* Fix TH bracket portability for GHC < 9.0 / template-haskell < 2.17
RenderRoute.hs: TemplateHaskellQuotes does not enable nested $(...)
splices inside brackets on GHC 8.8/8.10 — they parse as the ($)
operator and fail with cross-stage Lift errors. Use the full
TemplateHaskell pragma.
ParseRoute.hs: brackets are monomorphic Q before template-haskell
2.17, which conflicts with buildInlineParseClauses' Quote
polymorphism. Keep the bracket but run it through th-compat's
unsafeQToQuote, which is safe because the bracket draws no hygiene
names (its only binder comes from a splice).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address L0neGamer review: test layout + spec completeness
Layout: runtime/spec modules now live in the same folder as their
setup-data modules. Each multi-module test family is one directory
(NestedDispatch/, SplitSubsite/, FallthroughMatrix/,
FallthroughDispatch/, ParamDefaultSplit/, ParamFocusSplit/,
ParamSubsite/, SubsiteFallthrough/, SubsiteOptsFallthrough/) with
Data/Resources alongside Runtime. Self-contained single-module specs
stay at the top level.
Spec additions:
- FallthroughSpec: parseRoute case for /foo (first parent matches
without falling through)
- SubDispatchAritySpec: assert the rendered error messages, not just
isJust (one full-message golden, one content check)
- FocusLeafConsSpec: assert the generated datatype's complete
constructor set (names extracted in the generating splice; a later
splice cannot reify a same-module spliced datatype)
- InstanceProbeSpec: arity-2 abstract-instance case, and an
instance-at-concrete-argument case (a unifier counts as could-match);
note on why an unapplied-constructor instance is unrepresentable
Comment cleanups: internal review labels (R10, T5, ISSUE 11, '#1
regression') rewritten as behavior descriptions; reviewer-relative
framing neutralized; commented-out trailing-nest routes (inherited
from the old Hierarchy.hs) dropped; stale TODO removed; 'Expecte'
typo; ParamSplitSubNested now explains why the fragment entry point
takes structured Cxt/TyArgs instead of a name string; Hierarchy.Nest
documents the canonical low-level split recipe and points at the
guide.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address review: total Match-building + DispatchPhase enum
ParseRoute: split buildInlineParseClauses into a thin Match->Clause
wrapper over a new buildInlineParseMatches that yields [Match] directly,
so a parent folds its children's alternatives straight into its case.
Removes the partial clauseToMatch and its error fallback (an unreachable
shape assertion that a reader still had to prove dead).
Dispatch: replace the envWrap :: Exp -> Exp field with envPhase ::
DispatchPhase (TopLevelPhase | NestedPhase) interpreted by a single pure
wrapForPhase. The field previously admitted any Exp -> Exp though only
id / Just-wrap were ever valid; the enum makes the two phases the only
inhabitants. One askWrap helper feeds both the leaf and parent bodies.
Both are the tag-over-function / total-by-construction direction already
taken with SameSpliceNestedInstances.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address review: close MonadFail gap, drop boolean blindness, doc fixes
- Parse.hs: pieceFromString is now MonadFail and routes Dynamic pieces
through dropBracketM, so an unclosed bracket in a route piece (e.g.
"#{Foo") surfaces as an attributed splice error instead of bottoming
out in the pure dropBracket's error. Closes the gap left by the
earlier piecesFromString MonadFail conversion.
- RenderRoute.hs: discoveryMode now takes TyArgs directly and calls
hasTyArgs internally, instead of a bare Bool. Every call site passed
`hasTyArgs tyargs` anyway; this removes the footgun the haddock had to
warn about (don't confuse the Bool with NoTyArgs). Callers updated in
RenderRoute, Dispatch, ParseRoute, Internal/TH; test updated to pass
TyArgs (toTyArgs []/[..]).
- RenderRoute.hs: delete commented-out pre-bracket delegatingBody code.
- Class/Dispatch.hs: UrlToDispatch haddock no longer implies the url
value is dispatched on; document that dispatch is path-based and the
url argument only selects the instance / carries the constraint.
- Routes/Class.hs: document that parseRouteNested and renderRouteNested
are deliberately not inverses (parse recovers only the fragment, never
the ParentArgs).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Make mkNestedSubDispatchInstance take [ResourceTree String]
The hand-written subsite-splitting recipe previously forced users to
convert the quasi-quoter's [ResourceTree String] to [ResourceTree Type]
themselves via `map (fmap (parseType . dropBracket))`, which meant
re-exporting the *partial* parseType/dropBracket from the blessed
Yesod.Core.Dispatch and running them inside the splice (a malformed type
bottomed out as a GHC panic).
Push the parse boundary inward: the public mkNestedSubDispatchInstance
now takes [ResourceTree String] and calls parseResourceTypes internally
(dropBracketM >=> parseTypeM, in Q), so a bad type fails the splice with
an attributable error and users never touch the partial primitives.
Because parsing lives in Yesod.Routes.Parse (above Yesod.Routes.TH.Dispatch
in the import graph), the String-taking entry point lives in
Yesod.Core.Internal.TH, delegating to the now-exported Type-taking worker
mkNestedDispatchInstanceWith. The internal mkYesod path calls that worker
directly with already-parsed resources (no double parse). Dropped the
parseType/dropBracket re-exports from Yesod.Core.Dispatch.
Recipe in docs + the three split-subsite test modules updated to pass the
String resources directly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Arity-check the recipe's top target; keep the pure check un-ignorable
mkNestedDispatchInstanceWith only arity-checks nested children, so the
hand-written subsite-splitting recipe (mkNestedSubDispatchInstance ->
worker) built its top target's instance head with no arity guard. A
recipe pairing a parameterized subsite with an unparameterized target
datatype would apply the subsite's type args to a kind-Type head and hit
a cryptic kind error instead of the actionable arity message.
Close the gap in the recipe wrapper, where target + tyargs are known:
resolveRouteCon + assertNestedSubArity before delegating. No-op when the
datatype is out of scope (unknowable arity) or the arities match, so the
existing valid recipes (incl. the parameterized arity-1 case) are
unaffected.
On the parse-don't-validate angle: a ValidatedSubHead token threaded
through the worker was considered but rejected. The arity message needs a
"container" name (foundation subsite vs parent route) that differs per
call site and lives in the caller, not at the single head-construction
point, so a global token would degrade the message. The check is already
un-ignorable in practice: checkNestedSubArity (pure, Maybe-returning) is
called only by assertNestedSubArity (Q (), for effect); checkNestedSubArity
stays exported solely for its unit test.
A negative test (mismatched top target -> splice failure) isn't
expressible without a compile-fail harness; the pure logic is covered by
SubDispatchAritySpec and the happy-path wiring by the recipe modules.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Pre-merge cleanup: drop dead imports/export/param, fix haddock
Tidy warts surfaced by a -Wall + -Wunused-imports sweep of the branch's
changed modules (the package builds with -Wall, so these would warn on a
clean recompile):
- RenderRoute: remove four dead imports (Yesod.Core.Class.Dispatch,
Yesod.Core.Handler, Data.Proxy, Yesod.Core.Class.Yesod) — referenced
only in haddock prose, never as code; and delete nullifyWhenNoParam,
an exported helper with zero remaining call sites (the discoveryMode/
childCxt rewrite replaced it). Drop its re-export from Routes/TH too.
- RenderRoute: drop the unused RouteOpts parameter from the internal
mkToParentRouteInstances (-Wunused-matches), and fix a stray double
space in its signature.
- Dispatch: remove the now-dead Web.PathPieces import; fix an unbalanced
TH name-quote in nestedDispatchCall's haddock that misrendered.
- Class/Dispatch: narrow the open Network.HTTP.Types import to the two
symbols actually used (status301, status307), matching master's style.
- Routes/TH/Internal: drop unused RecordWildCards and ViewPatterns
pragmas.
Changed modules now build clean under -Wunused-imports; full suite green
(tests + test-routes).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Remove duplication in route codegen; clarify names
Follow-up to the pre-merge review: factor out genuine duplication and
sharpen a few names, with no behavioral change (tests + test-routes
green, warning-clean under -Wall -Wunused-imports -Wname-shadowing).
RenderRoute:
- Hoist the verbatim-duplicated `mkPieces` and `isDynamic` helpers (one
copy each in mkRenderRouteClauses and mkRenderRouteNestedClauses) to
single module-level definitions.
- Factor the duplicated child-deriving-context rule into `childDerivCxt`;
both mkRouteConsOpts and mkRenderRouteNestedInstanceOpts now delegate.
- Rename the focused-children generators mkRouteConsOpts'/mkRouteCon' to
mkFocusedChildCons/mkFocusedChildCon and document why they are
deliberately NOT merged with the in-module mkRouteCon: the latter must
honour the focus gate + InlineCompat branch, while here every tree is
already inside the focused target, so applying that gate would wrongly
skip grandchild instances.
Dispatch:
- Rename route'/route -> thisRoute/fullRoute (and the subsite lambda to
routeBuilder): thisRoute is this route's own constructor applied to its
pieces; fullRoute wraps it in the enclosing parent constructors.
ParseRoute:
- Factor the repeated `_ -> Nothing` trailing clause into top-level
`missingRouteClause`, and the repeated `applyTyArgs (ConT (mkName n))
origTyargs` into a local `targetTypeFor`.
- Drop the thin (unexported) `generateParseRouteClausesInline`
pass-through; its sole caller now uses `buildInlineParseClauses`
directly, with the relevant haddock folded in.
- Inline the single-use `recordName` into `recordNameIfNotInstance`
(when not . member -> unless).
Core/Internal/TH:
- Factor the duplicated parseYesodName-then-fail into `parseYesodNameQ`.
- Replace the recursive `findNested` local with a list comprehension.
- Rename the `parseRoute` local to `parseRouteDec` (matches its
dispatchDec/resourcesDec siblings; clears a name-shadow of the imported
parseRoute).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* FocusLeafConsSpec: pin the constructor set with a total case
Addresses L0neGamer's review question ("can we assert these are the only
constructors of FocusNestR?"). The name-list reflection that previously
answered it (lift the Con names, compare at runtime) is replaced by a
total `describeFocus :: FocusNestR -> String` case, with the module
escalating -Werror=incomplete-patterns. This pins the generated set at
compile time and more cleanly:
* a missing constructor fails the pattern reference (name not in scope);
* an extra constructor makes the case non-exhaustive, which the
-Werror turns into a build failure.
Verified the guard actually bites: dropping the FocusShowR arm fails the
build with "Patterns of type 'FocusNestR' not matched: FocusShowR _".
Drops the now-unneeded lift/conName/SigD-ValD splice machinery and the
Language.Haskell.TH.Syntax (lift) import.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* InstanceProbeTypes: note why L0neGamer's under-applied cases can't be tested
L0neGamer suggested three probe cases. The concrete-applied one
(instance Probe (HasInstInt Int)) is already covered. The other two —
instance Probe HasInst1 (unapplied arity-1) and instance Probe
(HasInst2 a) (partially-applied arity-2) — are not representable: Probe
is Type-kinded like the real nested-discovery classes, so a head of kind
Type -> Type is kind-rejected at its definition site ("Expected a type,
but ... has kind * -> *"). Verified by adding both to this module and
watching GHC reject them.
Generalize the existing unapplied-constructor note to name both
under-applied shapes explicitly, so the code answers the review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* InstanceProbeTypes: add poly-kinded ProbePoly + under-applied probe tests
L0neGamer suggested probing instances written at under-applied heads
(instance at a bare arity-1 constructor, and at a partially-applied
arity-2 one). Those aren't representable against the Type-kinded Probe —
GHC kind-rejects the instance head — so make them representable with a
poly-kinded `class ProbePoly (a :: k)` and pin what the probe does:
- PolyFull (instance ProbePoly (PolyFull a), fully applied) -> True
- PolyUnapplied (instance ProbePoly PolyUnapplied) -> False
- PolyPartial (instance ProbePoly (PolyPartial a)) -> False
The False results are the point: nestedInstanceExists saturates the
datatype to its own arity (a kind-Type head, PolyUnapplied a /
PolyPartial a b) before querying isInstance, so an instance at the bare
or partially-applied constructor is a *different* head and does not
match. The probe queries at the fully-applied head specifically and
isn't fooled by an under-applied instance. Verified empirically (the
expectations were confirmed by the run, not assumed).
Generalize the existing note to cross-reference ProbePoly, and merge the
duplicate import. test-routes: 107 examples, 0 failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* InstanceProbeSpec: pin why the probe must saturate to full arity
Direct isInstance queries at hand-built heads, documenting the matching
boundary that justifies fullyApplyType saturating to the datatype's full
arity:
- ProbePoly (PolyPartial Int) [one arg short, kind Type->Type] -> True
- ProbePoly (PolyPartial Int Int) [full, kind Type] -> False
- ProbePoly (PolyUnapplied Int) [kind Type] -> False
The partial instance ProbePoly (PolyPartial a) matches *any* one-arg
application, so a probe that stopped one argument short would be
spuriously fooled by it; saturating to the full arity (PolyPartial a b,
kind Type) is what avoids that. test-routes: 110 examples, 0 failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* FallthroughSpec: add a /foo dispatch case
Per L0neGamer's review note. The Dispatch block round-tripped only "/";
add "/foo", which dispatches to FirstFooR's index handler (FooIndexR).
test-routes: 111 examples, 0 failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Hierarchy/Nest: use canonical mkYesodDataOpts focus entry point
Per L0neGamer's review: once this PR merges, mkYesod/mkYesodData are the
canonical ways to use yesod, so a split fragment should model them.
Replace Nest.hs's three low-level focused builders
(mkRenderRouteInstanceOpts + mkRouteAttrsInstanceFor +
mkParseRouteInstanceFor) with the single bundled
mkYesodDataOpts (setFocusOnNestedRoute "NestR" defaultOpts)
"Hierarchy" hierarchyResources
which emits exactly the same focused render/attrs/parse instances (its
data half is [parseRouteDec, renderRouteDec, routeAttrsDec], honoring the
focus). This mirrors the existing Hierarchy.Nest3 fragment.
No coverage lost: the sibling fragments Admin, Nest2 and Nest2.NestInner
still drive the underlying per-instance builders directly, so the
route-level generators mkYesodDataOpts bundles stay exercised. Updated
the module haddock to say so. hierarchy spec: 111 examples, 0 failures,
warning-clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix GHC <9.0 CI: version-gate concrete-arg reifyInstances probe
The InstanceProbeSpec HasInstInt case probes the abstract head
`HasInstInt a` against the concrete `instance Probe (HasInstInt Int)`.
GHC 9.0+ reifyInstances returns unifiers (so it matches → True), but
GHC 8.8/8.10 do not unify a bare type-variable query head against a
concrete-argument instance (→ False), failing the lts-16/lts-18 CI jobs.
This concrete-argument shape never arises in real codegen (nested
discovery instances are always emitted at fully-abstract parameters,
resolved identically on every supported GHC), so CPP-gate the expected
value per GHC version to pin the observed reifyInstances divergence
rather than a behaviour the codegen depends on. Update the HasInstInt
doc comment to match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Daniel Powell <dtpowl@gmail.com>
Co-authored-by: Daniel Powell <3739511+dtpowl@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Barry Moore II <3086255+chiroptical@users.noreply.github.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Daniel Powell <dtpowl@gmail.com>
Co-authored-by: Daniel Powell <3739511+dtpowl@users.noreply.github.com>
Co-authored-by: Barry Moore II <3086255+chiroptical@users.noreply.github.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
5 participants
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.
Nested Route Separation
We have almost 2,000 total routes in our codebase now, with 1,100 of them on the top-level.
We have been leveraging nested routes to organize the routes and make certain things easier - namely, reducing the overall size of the
Route WebAppwhich makes pattern matching more efficient in compilation.However, compiling
mkYesodDatafor our application is a large bottleneck.Compilation takes a significant amount of time and much work must be redone, even when most of the routing information could have been saved.
One thing that would significantly help is the ability to separate out the sub-route datatypes, generate the instance for these separately, and refer to them elsewhere.
Another thing that would help is having a finer grained
YesodDispatchfacility - allowing us to, in tests, specify more precisely which parts of the route structure we actually need to deal with.Currently, we need almost 7k modules in order to compile a test that refers to
YesodDispatch, since this brings in the transitive dependencies of every web handler.With a finer grained
YesodDispatch, able to refer to route fragments, we'd be able to avoid depending on more of the website than we really need to.We expect this to yield significant benefits when we move more testing infrastructure to buck2 - right now, changing anything used by any part of the web app would require running all tests that exercise anything in the webapp!
With more granular
YesodDispatch, we gain the ability to only run tests on parts of the app that actually use the route structure in question.This PR is a breaking change, and so will be
yesod-core-1.7.0.0. When I tried this branch on our package, it required 0 modifications to upgrade - most of the breaking changes here are breaking "advanced power user" type functions, like the ability to customize theMkDispatchSettingsto have highly custom route behaviors. We don't use any of those, and I have no idea how widespread their use is. I will admit to finding them very difficult to use and adapt, which is part of why I ended up scrapping their use in theParseRoutegeneration (the class isn't even used).For the most part, though, all prior behavior remains unchanged. The performance of routing may be slightly worse -
YesodDispatchNestedreturns aMaybe Application(but with theRequestpulled out - only the "send response" side is in the Maybe) to support fallthrough. And if you enable fallthrough, then the app will do more checking (proportional to the number of routes and nested routes) rather than early-404ing. But IMO for perf, we're better off coming up with a trie-based router than trying to optimize this one much more.For an example of the testing facilities this provides, see parsonsmatt/hspec-yesod#7 - particularly, this file has an introduction to how the tests are separated and defined.
Fix for #1880
Before submitting your PR, check that you've:
@sincedeclarations to the Haddocks for new, public APIsAfter submitting your PR: