-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nested WAI Applications under Scotty Routes #233
Conversation
Looks like some of the apt-get CI commands are failing? |
examples/nested.hs
Outdated
scottApp = scottyApp $ do | ||
|
||
get "/" $ do | ||
html $ mconcat ["<h1>Scotty, bean me up!</h1>"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ocramz updated to beam
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think introducing 'nested' is a nice addition. @sordina could you add a test for this as well? Edit: and a Changelog entry as well please.
Noticed this warning when compiling:
|
yes we have a ticket and a PR for this already!
…On Fri, 7 Jul 2023 at 09:38, Lyndon Maydwell ***@***.***> wrote:
Noticed this warning when compiling:
Web/Scotty/Internal/Types.hs:168:5: warning: [-Wnoncanonical-monad-instances]
Noncanonical ‘return’ definition detected
in the instance declaration for ‘Monad (ActionT e m)’.
‘return’ will eventually be removed in favour of ‘pure’
Either remove definition for ‘return’ (recommended) or define as ‘return = pure’
See also: https://gitlab.haskell.org/ghc/ghc/-/wikis/proposal/monad-of-no-return
|
168 | return = ActionT . return
—
Reply to this email directly, view it on GitHub
<#233 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNBDKBAD5UMCCFWBF5M4T3XO64F7ANCNFSM4JEOOT6Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ocramz I've added a spec that matches the simple example from |
wyt @fumieval is this good to merge? |
_ <- liftAndCatchIO $ app r (\res -> putMVar ref res >> return ResponseReceived) | ||
res <- liftAndCatchIO $ readMVar ref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this catch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems 'app' could put a landmine inside the MVar so.. I think so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good PR I think
_ <- liftAndCatchIO $ app r (\res -> putMVar ref res >> return ResponseReceived) | ||
res <- liftAndCatchIO $ readMVar ref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems 'app' could put a landmine inside the MVar so.. I think so?
@sordina once you're able to add an entry to the Changelog I'll gladly merge. Thank you! |
@ocramz Added a changelog entry under the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@sordina Thank you! all good 👍 |
This PR adds a new
nested
handler that allows you to place an entire WAI Application under a Scotty route.Let me know if this implementation fits!
Simple Example:
Reference:
describe nested
inScottySpec.hs
.examples/nested.hs