Make previews entry endpoint configurable

[juanmanuelramallo]

Feature request

Allow users to configure the entry endpoint for previewing components.

Motivation

Started here https://github.com/github/view_component/pull/324#discussion_r419091598.

Also several gems allow users to mount their views in a given endpoint, for instance Split, Flipper, Sidekiq:

# config/routes.rb
mount Sidekiq::Web => '/sidekiq'

Although, we may not need to let the users to mount the previews like this, but instead we can just provide a configurable attribute for the base endpoint in the view components config, and use it if the previews are enabled.

[jonspalmer]

Curious what others think but I think I lean towards requiring the user of the gem to choose the mount point in routes.rb. Its would be a 'breaking' change for anyone using previews today. Given the folks should be thinking about securing these pages and whether they are on/off in production etc. routes.rb is the right place for those decisions.

It's not what ActionMailer previews does but IMO this is a place where we should be prepared to break from that example. @joelhawksley and others?

[juanmanuelramallo]

Following the same line of thought, what should happen when the user have show_previews disabled and the mount point is in routes.rb (let's say at the top of the file)? Should we not mount anything, despite of the user explicitly mounting the preview routes there?

Your point makes sense to me, but from an implementation perspective, just adding a config attribute is way simpler, I think. If we want to allow users "mount" the previews, we need to either implement a fully compliant Rails::Engine or go with a Rack app.

[jonspalmer]

I'm proposing we eliminate show_previews as a config option and rely on the user mounting the engine or not. You can make routes.rb conditional base on Rails Env or any other rules (and the user of the gem could create their own config param if they wanted to to control it via config/production.rb, config/development.rb etc.)

If we want to allow users "mount" the previews, we need to either implement a fully compliant Rails::Engine or go with a Rack app.

I don't have much experience with what is involved there. We already have an Engine what does it take to add routes to it? Reading up on the docs here: https://guides.rubyonrails.org/engines.html It looks like we'd add config/routes.rb to the engine. I think it probably also solves the ApplicationController/url_helper problem you have in #330

[juanmanuelramallo]

Yeah that looks good! So in order to have view component previews, users should now mount the preview engine. And if they don't want it on prod they can just always wrap it in an if clause, right?

I'm not sure it will also solve the url_helper problem, but if we end up removing the Rails::ApplicationController as the parent controller, I'm sure we'll get it solved.

How should we proceed then?

[jonspalmer]

Yup wrap in an if condition or you can supply a constraint on the call to mount

WRT Rails:: ApplicationController see the details here: https://guides.rubyonrails.org/engines.html#app-directory

It looks like the Engine defines an ApplicationController and later in the docs there are examples of using url helpers.

In terms of proceeding. I'd like opinions from others. @joelhawksley etc. This would be a 'breaking' change but I think the benefits out weight the pain.

[joelhawksley]

👋 I think I prefer the approach you're taking in #330 @juanmanuelramallo. It keeps our implementation closer to ActionMailer previews ❤️

[jonspalmer]

@joelhawksley The challenge I have is that ActionMailer previews are IMO pretty badly implemented. If I'm including a gem that brings routes with it I should configure those routes in routes.rb. As the application owner that allows me to keep all my routing in one place.

As it stands there are two configuration variables I need to think about to makeVC previews work vs a single mount point to configure. Routes.rb also has a lot more flexibility in terms of when to 'show' previews that a simple "show_previews" boolean doesn't allow.

I'm all for following Rails patterns but only when they're good ones and ActionMailer previews don't meet that for me. Instead I suggest we should be inspired by hugely popular gems like sidekiq etc. that configure via mounting an engine.

As a side benefit using true routing in an engine we think gives us a bunch of structural advantages to the preview code, view helpers etc.

[juanmanuelramallo]

I'm ok if we decide to close #330 and update this issue.

I'm ok with either of both solutions to be honest, I just find #330 to be the easier one, but I don't want to be lazy and just roll this out because it's easier.

So I think @joelhawksley it's your call here, either to move forward with #330 or to update this issue and mention that the solution must be a mountable engine.

[joelhawksley]

@jonspalmer that's a fair point. Has this issue been raised on Rails at all?

While it doesn't look like Rails Conductor is going to be part of 6.1, I could imagine it providing hooks for this kind of internal functionality. Perhaps we table moving to an engine until we have more clarity there?

Generally, I view what we're doing with ViewComponent as more closer to Rails than not, unlike Sidekiq. Even if the ActionMailer implementation isn't ideal, I value consistency a lot.

That being said, I don't feel incredibly strongly about this issue. If either of you do, I will defer to your decision. But if we do decide to go with a mountable engine, we'll need to consider what timing / deprecations will be appropriate.

[juanmanuelramallo]

Your call then @jonspalmer, how would you want to proceed?

Personally, I prefer the configuration attribute introduced in #330 so that we keep it simple (implementation and usage).

But I see that mounting the previews in the routes file of a project might be clearer for the final user and would give them more control.

[jonspalmer]

@joelhawksley I hear you on the consistency. My sense is that ActionMailer previews is a pretty weird wart on the Rails framework and one that hasn't had a lot of attention recently. I've also found that it has relatively limited value once you have an app that is sending a significant volume of email with any complexity. I certainly seen apps where we wrote our own versions equivalent things.

That said I'm ok going with #330. I'm likely to use Storybook for 'previews' anyway as there are more feature with that approach. If that allows us defer a breaking change decision until later I'm ok with it.

P.S. What is Rails Conductor? I don't have it on my radar.

[joelhawksley]

@jonspalmer here is the conductor PR: https://github.com/rails/rails/pull/35489. I'm not sure what the plan is for it at this point.

I'd be interested in documenting Storybook as a solution for more advanced previews.