Introduce middleware for request dispatch and console rendering #69
Conversation
|
@guilleiguaran Please, take a look when you have the time :) @rafaelfranca, I hate to put one more thing on your plate, but since you weren't happy with the template situation, you may be interested in that :) |
|
@gsamokovarov this looks great in my opinion and as you said is a excellent solution for the template situation, I'll like to get a couple more of reviews before of merge this 😃 |
gsamokovarov
force-pushed
the
introduce-middleware
branch
from
1d8a74e to
4a1f31b
Compare
| end | ||
|
|
||
| ActiveSupport.on_load(:action_controller) do | ||
| ActionController::Base.class_eval do |
There was a problem hiding this comment.
Can we use include instead of class_eval?
There was a problem hiding this comment.
Yep, will do that.
gsamokovarov
force-pushed
the
introduce-middleware
branch
2 times, most recently
from
743dfe4 to
11aaaaf
Compare
| module Helper | ||
| # Communicates with the middleware to render a console in a +binding+. | ||
| def console(binding = nil) | ||
| request.env['web_console.binding'] ||= (binding || ::Kernel.binding.of_caller(1)) |
There was a problem hiding this comment.
I was thinking of putting render-like semantics here. E.g. raise if you are to have two consoles, instead of getting surprised if the console is in an unexpected binding. @rafaelfranca wdyt?
It may happen that application code sets web_console.binding first and then the app fails. The console pages will spawn a console in an unexpected binding then.
It can be shared between the two at the moment.
gsamokovarov
force-pushed
the
introduce-middleware
branch
from
1b5eca2 to
8a5d86e
Compare
Once ActionView::Helpers is included into ActionView::Base, including more modules into it, won't make the methods findable by ActionView::Base instances.
|
@rafaelfranca, @chancancode Do you guys have the time take a final look? I really wanna bite the bullet and merge this so I can release 2.1 around new year :) We can still polish a lot of stuff after the merge, but I think this is the push we need. |
|
I'm gonna merge it as is. Guys, if you still have comments about it, leave them here or open an issue and I'll look them up. |
Introduce middleware for request dispatch and console rendering
|
👍 |
Having to copy and maintain the Rails templates in sync was trouble. Having
ActionDispatch::DebugExceptionsdispatching requests for Web Console was out of place. Having an after action in the controller, that avoids the render process was bad. It was clear that we need a better solution.I'm proposing to solve all that with a smartly placed middleware between
ActionDispatch::DebugExceptionsandActionDispatch::ShowExceptions. Now every page can have an attached console, as long as the application passed a binding (or an exception to extract a binding from) down to the application.This makes the
consoleview and controller helpers a really thin wrapper that communicates with the middleware. Because we can only attach one binding or one exception, we don't need to handle how to avoid double console rendering.The error pages are a bit special, because we have a stack of bindings and we can change them. This is done with extra JavaScript working with current
ActionDispatch::DebugExceptions. The only drawback of this is that it works only with Rails 4.2 templates, but I think this is a good compromise. If this is a requested feature for older Rails installation, we can implement it for them too– it would take only a little bit of JavaScript.Last and not least, testing the middleware is quite easier, so it can give us more confidence about the console.