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

Inconsistent overriding of previously-defined helpers #1213

Closed
mwpastore opened this issue Dec 7, 2016 · 2 comments
Closed

Inconsistent overriding of previously-defined helpers #1213

mwpastore opened this issue Dec 7, 2016 · 2 comments
Labels
Milestone

Comments

@mwpastore
Copy link
Member

There's a slight inconsistency with the order in which Sinatra applies helpers when those helpers may have been previously defined. This is probably best explained with an example:

module HelperOne
  def one; '1' end
end

class MockApp < Sinatra::Base
  helpers do
    def one; nil end
    def two; nil end
  end

  helpers ::HelperOne do
    def two; '2' end
  end

  get('/one') { one }
  get('/two') { two }
end

One might reasonably expect one of two outcomes from the above:

  1. /one returns "1" and /two returns "2" (i.e. the later-defined helpers override the previously-defined helpers)
  2. /one returns nil and /two returns nil (i.e. the previously-defined helpers are "sticky" and cannot be overridden)

But what actually happens is:

  1. /one returns nil and /two returns "2"

So previously-defined helpers can be overridden in the block, but not in an included helpers module. This is inconsistent and confusing. To fix it, let's prepend the module(s) instead of include-ing them.

mwpastore added a commit to mwpastore/sinatra that referenced this issue Dec 7, 2016
mwpastore added a commit to mwpastore/sinatra that referenced this issue Dec 7, 2016
@zzak
Copy link
Member

zzak commented Dec 14, 2016

I'm not sure extent the affect of changing to prepend could have on existing applications, so my gut is to hold off on this.

@namusyaka
Copy link
Member

Looks reasonable.

jkowens pushed a commit to mwpastore/sinatra that referenced this issue Mar 18, 2020
@namusyaka namusyaka reopened this Nov 4, 2020
namusyaka added a commit that referenced this issue Nov 11, 2020
Revert "Use prepend instead of include for helpers. Fixes #1213"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants