Named route helpers should be explicit in which route set is being used

[afred]

rationale: I am writing an engine that use Blacklight views for it's defaults. Within Blacklight, there are views and partials that call named route helpers without prefixing the main_app route set, e.g. bookmark_path.

When testing my engine, I was getting undefined local variable or method 'bookmarks_path' because it was trying use the routing proxy for my engine, instead of the routing proxy forthe host application.

Rails provides helper methods to the different routing proxies. The default name for the host app's is main_app, and the default for an engine is the same as the engine name, i.e. for MyEngine, you can access the routing proxy with my_engine. These helpers are available in controllers and views.

I think the incompatibility issue I was having with Blacklight could be alleviated if Blacklight were to explicitly use main_app.bookmarks_path in it's views and controllers, instead of just bookmarks_path.

Done when guideline is accepted or rejected, after discussion.

[afred]

@projecthydra-labs/hydra_plugins_wg thoughts?

[botimer]

I think calling the helper on an object is sane (rather than a method floating in the calling context), but I wonder if main_app is actually the right answer. It may be that that resolution gets flaky (i.e., while actively developing Blacklight). Maybe not.

If it does get flaky, It should be pretty easy to configure an object instance of a given name (maybe main_app, but probably not, to avoid any clobbering) and, within the engine, default it to Blacklight's router, but as an app initializer (possibly generated), allow override to the host app.

[afred]

From my experience, while developing your own engine, you would use the main_app helper to get the set of routes that are defined within the host application, i.e. with Rails.application.routes.draw. Meanwhile, you use the my_plugin helper to get the set of routes defined in MyPlugin::Engine.routes.draw.

The bookmarks_path url helper is created from within Blacklight, but the route itself is a result of resourceful routing (via Rails), which means Blacklight is not defining it within Blacklight::Engine.routes.draw. Therefore, the bookmarks_path url helper should explicitly use main_app.bookmarks_path to avoid any ambiguity with other engines.

The helper method's name main_app is just the default. It can probably be changed, but probably shouldn't be. I think it's a convention we could stick with.

Contrast that with the Blacklight route for search history. Since this is defined within Blacklight::Engine.routes.draw, it is a part of the Blacklight engine's route set, and should be called using blacklight.search_history_path, which Blacklight is already doing.

[mbarnett]

Contrast that with the Blacklight route for search history. Since this is defined within Blacklight::Engine.routes.draw, it is a part of the Blacklight engine's route set, and should be called using blacklight.search_history_path

Relatedly: are there any more extension-friendly/ composable patterns we can use or recommend over this?

An example would be, say, FooBar plugin calls foobar.download_path(generic_work) in many views & partials, which is fine right up until the point where we needed to override path generation for a few special cases -- our repository has some snowflake Works where the download_path is external, so we have to detect them and do something different.

Different gems seem to approach this in different ways -- Devise makes the router_name a dynamic part of its config which defaults to main_app, but if you need to compose it with some other engine you can swap the router over to the engine's route proxy, instead.

Support for this, or even just some method_missing magic to forward invocations of foobar.download_path to main_app if main_app responds_to? an override, might be a potential use-case for #22

[afred]

My main motivation for suggesting this guideline was because I ran into an issue where Blacklight was calling url helper without specifying the main_app routing proxy. This works fine within Blacklight's tests, but when I tried to render the same Blacklight view from my engine's tests, it broke because the default routing proxy was not main_app, and the url helper was not defined.

Rails Guides warns of issues like this:

If a template rendered from within an engine attempts to use one of the application's routing helper methods, it may result in an undefined method call. If you encounter such an issue, ensure that you're not attempting to call the application's routing methods without the main_app prefix from within the engine.

The proposal of this guideline was to just reinforce what Rails Guides is already saying we should do.

For @mbarnett's use case, I think that would be a separate guideline that says something like:

A plugin may dynamically change the name of it's routing proxy by doing __.

[jcoyne]

I see this as a bug in Blacklight and not something that ought to concern a plugin. That said, if you ever need to reference routes from outside your plugin,(main_app or other plugin) you must be specific with the namespace.

[afred]

I agree, but the purpose of the guideline is to say "don't let your plugin cause this bug", which a plugin developer could accidentally do, since it would likely pass within the context of the plugin's test suite.

[jcoyne]

Blacklight is not behaving as plugin in this case ( it's adding routes to the main app). Is there another non-Blacklight example you can provide?

[afred]

No, I don't have an explicit case for non-Blacklight. However, the same type of bug will come up if plugins are trying to interact with each other in the same way my plugin is trying to do with Blacklight, which is to say.

1. PluginA adds routes to the main app, which have URL helpers available via main_app routing proxy. Let's say the URL helper ismy_model_url.
2. PluginA includes a view, say my_view.html.erb that reference my_model_url without explicitly using the main_app routing proxy. No errors are raised.
3. PluginB renders the my_view.html.erb view from one of it's own routes. An error is raised because the implicit routing proxy is plugin_b, but the URL helper method my_model_url is not defined for that routing proxy, but rather the main_app. This can be avoided if PluginA simply does not rely on the implicit routing proxy, and specifies main_app instead.

Again, my main motivation was to just re-state what the Rails guides are already saying here.

Also, this may be too much of a one-off to care about. It's just something that I ran into, so I thought I'd suggest it.