Skip to content
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

is Mux.jl an appropriate home for HTTP/Revise glue? #135

Open
clarkevans opened this issue Dec 28, 2020 · 7 comments · May be fixed by #161
Open

is Mux.jl an appropriate home for HTTP/Revise glue? #135

clarkevans opened this issue Dec 28, 2020 · 7 comments · May be fixed by #161

Comments

@clarkevans
Copy link

clarkevans commented Dec 28, 2020

@cmcaine -- would Mux.jl be a good place to have a middleware component that adds Revise.jl glue? See JuliaWeb/HTTP.jl#587

If we're promoting Mux.jl as an application developer stack, having Revise functionality out-of-the-box is kinda important. For now, it's just a side script in the application I have, but it'd be better if this were shared with others.

@clarkevans
Copy link
Author

clarkevans commented Dec 29, 2020

I've gotten my baby TodoMVC.jl to work with this glue. It's not great, I still think Mux.jl may be the most appropriate place for targeting this sort of developer oriented workflow. The bulk of the work happens in live_server.jl which probably needs a different name, but if it could be integrated here, that might be useful.

@cmcaine
Copy link
Collaborator

cmcaine commented Dec 29, 2020

I think that having Mux work with Revise is worthwhile, but I'm not totally sold on this approach. I don't really like that we're closing and re-opening the socket, either, though maybe that's fine.

Re-evaluating the @app expression should be sufficient for Mux.serve to use the latest version of the code. Perhaps we can do something with that?

Looking at the revise docs, it seems like Revise.includet doesn't evaluate data definitions again if they change in the file, it just evaluates them on the first go around, then updates only methods. Which would explain why the top-level statements generated by the macro aren't being re-evaluated when they change. There might be something we can do there with Revise.track directly (though my initial attempts at that aren't succeeding).

@clarkevans
Copy link
Author

I think that having Mux work with Revise is worthwhile, but I'm not totally sold on this approach.

Awesome. We're on the same page. I don't like the opening/closing of the socket either. Frankly, there's parts of the code I don't grok.

Which would explain why the top-level statements generated by the macro aren't being re-evaluated when they change.

With the existing TodoMVC, it did re-evaluate the @app macro, I think it only worked due to the clever if/else logic there. I've not tried an example using with Revise.includet.

@ndgnuh
Copy link

ndgnuh commented Mar 2, 2021

I managed to use Revise with this script:

using Pkg
Pkg.activate(@__DIR__)
using Revise: entr, revise
using MyApp


const t = MyApp.main() # call serve(app), nothing special


entr(String[], [MyApp]; all = true) do
	revise(MyApp)
end

@schlichtanders
Copy link

schlichtanders commented Jun 28, 2024

Stumbling upon this because also for me it seems that revise is not working with Mux.jl which is very unfortunate...

Are there any updates on makeing Revise work with Mux.jl?

@schlichtanders
Copy link

schlichtanders commented Jun 28, 2024

I just found a simple way: not to use @app.

I guess the advantage that Revise works out of the box like this is really huge.

module MyModule
using Mux: serve, mux, App
myserve(req) = req |> mux(
    ...
)

function main()
    serve(App(myserve), 1234)
end
end # module

Looks only very slightly alien ;-) but as Revise works and we have one macro less, I am actually quite happy with it.

@cmcaine
Copy link
Collaborator

cmcaine commented Jun 28, 2024

Ah, yes, that's a nice simple approach! Very nice 😃 Would you be happy to make a PR adding something like this to the readme?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants