grape icon indicating copy to clipboard operation
grape copied to clipboard

Helpers declared after routes aren't available.

Open nuclearsandwich opened this issue 13 years ago • 13 comments

When I have a Grape::API application and I declare a namespace, if I declare a route before the helpers block, those helpers aren't available to the route. If I declare the route after, they are. For example:

require 'grape'
class DamnDirtyGrape < Grape::API
    resource :foo do
        get :broke do
            # NoMethodError here.
            hello_helper
        end

        helpers do
            def hello_helper
                "Hello World!"
            end
        end

        get :works do
            hello_helper
        end
    end
end

In Sinatra, a simiilar small framework from which Grape takes inspiriation, both methods work

require 'sinatra'
get '/' do
    hello_helper
end

helpers do
    def hello_helper
        "Hello World!"
    end
end

It's not a major priority but given that Sinatra behaves this way it would be nice from a consistency perspective. If anyone is willing to mentor me a little for this I am more than willing to give it a stab myself.

Cheers and thanks! Steven!

nuclearsandwich avatar Jun 02 '11 23:06 nuclearsandwich

Helpers are included into endpoint when we define route (see build_endpoint). You could replace Grape::API.helpers with something like Grape::Endpoint.class_eval &block in order to achieve Sinatra-like behavior.

presskey avatar Jun 03 '11 22:06 presskey

Is this something Grape is interested in? If so I'll definitely do what I can.

nuclearsandwich avatar Jun 03 '11 22:06 nuclearsandwich

Sure, let's do it.

mbleigh avatar Jul 15 '11 03:07 mbleigh

This should also work for the new before and after functionality. Any progress on a patch?

mbleigh avatar Sep 02 '11 17:09 mbleigh

Unfortunately work got really busy right after posting this. But I've set aside this whole weekend for improving the open source tools behind my work projects. So I will take another look at it tomorrow and post the results.

nuclearsandwich avatar Sep 02 '11 17:09 nuclearsandwich

I added failing test cases here and here. Working on a fix now based on presskey's suggestion.

nuclearsandwich avatar Sep 04 '11 08:09 nuclearsandwich

I was spelunking lib/grape/api.rb and lib/grape/endpoint.rb and was getting worried that the only way to keep the helpers up to date for every new request would be to delay evaluation until just before the request itself is evaluated.

But it occurred to me that if we create the module helpers are eval'd into ahead of time instead of lazily, then it will be present no matter when the endpoint is built and thus subsequent helper {} blocks will be added to the same module as appropriate to scope. I'll have to do a bit more poking around to see if the same method can be applied toward the before and after blocks. What are your thoughts @mbleigh?

nuclearsandwich avatar Sep 04 '11 11:09 nuclearsandwich

So I think I've come up with the way that I would like to see this implemented. If you want to take it on @nuclearsandwich, I will gladly give you the shot, but otherwise I can try to work on it soon. Basically I want to create a new Grape::Context class that will replace the current settings stack. Each context is created whenever an API calls something that currently adds to the settings stack.

When a route is compiled, it uses the settings of all its context ancestors. This way if helpers or filters are added to an ancestor after the nesting is over, we can still utilize them. The only problem here is that I think we do need to do something to move the compile time or make the endpoint compilation happen more dynamically. Wouldn't it make sense then to make the call method not available on the class, but on an instance of the API class. We would provide a method such as compile! that would make an instance of the API that is compiled with all current context. This solves our problems without relying on run-time execution, just start-up execution of the compiling.

# config.ru
run MyAPI.compile!

What do you think?

mbleigh avatar Sep 04 '11 15:09 mbleigh

Based on what I saw, using a Grape::Context object to organize those settings seems like a good way to go. But I'm pretty sure that no compile!method will be needed thanks to Ruby's open classes and modules. The only problem could be the order of before and after filter execution but that's more of a behavior decision than an insurmountable problem. It looks like before and after filters currently run from least to most specific context. Is this behavior the we want to maintain?

nuclearsandwich avatar Sep 04 '11 22:09 nuclearsandwich

@mbleigh, have you started working on this? I'm taking a stab at implementing Grape::Context this weekend for GoGaRuCo.

nuclearsandwich avatar Sep 16 '11 16:09 nuclearsandwich

I am going to change this from "bug" to "feature request". We still want this :)

dblock avatar Feb 09 '13 21:02 dblock

Is this problem still going on? Looks to be solved.

namusyaka avatar Feb 01 '17 13:02 namusyaka

This issue is still valid:

class API < Grape::API
  prefix :api
  version 'v1', using: :path
  get '/' do
    puts hello_helper
  end

  helpers do
    def hello_helper
      'Hello World!'
    end
  end
end

Output:

undefined local variable or method `hello_helper' for #<#<Class:0x000055f26581eea0>:0x000055f265e1f9f8> (NameError)

dnesteryuk avatar Mar 29 '20 17:03 dnesteryuk