Enable overriding of Engine helpers #6496

Closed
ChristianPeters opened this Issue May 26, 2012 · 3 comments

Comments

Projects
None yet
3 participants

Nearly a year ago, Jonathan Rochkind pointed out that you cannot override Engine helpers. It's still unsolved in 3.2.3.

If you mount an engine into your app, you can override everything. Except helpers:

module MountedEngineHelper
  def foo_helper
    "Ha! You cannot override me!"
  end
end

module MyAppHelper
  def foo_helper
    "Oh dear. I think I will lose this battle! :("
  end
end

The MountedEngineHelper module seems to be included after MyAppHelper.

From my perspective, this an error in the Rails initialization process.

The only workaround that does not smell bad to me, is coming up with a new name (foo_helper_with_bar) and calling the original name (foo_helper) in there. This half aliased method chain means that I have to change the helper calls in my templates. Simply overriding the helper and calling super would be much cleaner in many cases.

Member

drogus commented May 27, 2012

From my perspective, this an error in the Rails initialization process.

I wouldn't say that the order of loading engines and application is a bug in itself. You can't guess what developer wants to load first and I'm sure that if default order was different, someone else would open another ticket saying that the order should be different.

That said, we've seen that problem and I introduced railties_order method, which allows you to decide which railtie will load first: 40b19e0. However, there are 2 problems with this method that I've found thanks to your ticket.

First problem is small bug that can cause Rails.application to show up twice in ordered_railties list. I fixed it here: 0e69705, but this was not causing any problems with order itself, as repeated application was added with lower priority, so it was ignored anyway.

The real problem is, while you now can set the order of loading engines, helpers method that loads all of the helpers for controller is basically ignoring the order of directories. The fix is here: 26ddf2f, but I'm not sure if I can apply it to 3-2-stable branch, because it will change the current order of loading helpers. It will probably have very little impact on most of the apps, though (to break, application would need to load helpers from more than one path and rely on overriding helpers based on alphabetical ordering of 2 files from different directories).

@josevalim @jeremy @spastorino what do you guys think about changing that?

Owner

spastorino commented May 28, 2012

+1 for the change in master
but -1 in 3-2-stable

Member

drogus commented May 28, 2012

@spastorino yeah, that will be probably the safest way

drogus closed this in e4aaac1 May 28, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment