trailblazer / cells

View components for Ruby and Rails.
https://trailblazer.to/2.1/docs/cells.html
3.07k stars 236 forks source link

Pull current controller's helpers into the class during request in the same way as ActionView::Base #379

Closed hazah closed 8 years ago

hazah commented 8 years ago

I've opened this pull request in the hopes of hashing out the recurring issues with providing cells with a stable access to existing framework helpers. This is still a work in progress. But I was hoping to get your input early. I am well aware of your distaste for [Rails] helpers, but having access to them consistently would mean a solid widget tool-kit capable of rendering any link/form without constantly hitting odd overrides from the initialization process.

At the moment I've some testing to do.

timoschilling commented 8 years ago

This should not be merged for two reasons:

  1. Every Rails related code will be removed from cells and will be go into cells-rails.
  2. You brake the core principal of Cells by exposing all helpers to the Cell. Even in a Rails related Cell, the Cell should have only the needed helpers.
hazah commented 8 years ago

This should not be merged for two reasons:

I'm looking for a discussion more than a merge here.

  1. You brake the core principal of Cells by exposing all helpers to the Cell.

I'm not so sure that it's that clear cut -- this issue had haunted this gem ever since I can remember.

I understand that Cells is built with proper encapsulation in mind. Helpers are, on the other hand, global request cross-cutting concerns & functions that extend the existing template framework. What happens now is that an enormous part of standard Rails is completely unavailable (polymorphic routes across engines). This, naturally, leads to more hacks, not clean code, which is a shame since Cells provide a lot of power except for when you work with links across an extensible (by engine plugins) web application.

Even in a Rails related Cell, the Cell should have only the needed helpers.

How do you propose to predict this case for the user without blocking them? As I have seen over the last few years, that issue has yet to have an adequate solution, meanwhile, the standard Rails request context is simply unavailable to the Cell without hacks. I maintain that helpers solve the problem of extending the capacity of the Base template itself, since some application extensions do belong to the global request context.

Ultimately users will abuse any tool, and helpers are almost always used inappropriately. Even Cells aren't immune. But there are cases when they are the proper solution. Internalisation, formatting, & tags are prime examples of extensions that belong to all cells if the underlying framework is already making this assumption. The same goes for the current user, the permission system, and if the application makes heavy use of it, the current category (as an extreme example of specific use case). Currently Cells only allow for a push model. This will enable both push and pull content models, a principle that works well for plugin systems.

Every Rails related code will be removed from cells and will be go into cells-rails.

That's fair. Decoupling is the name of the game. Anything rails specific can be part of its include and override the original methods instead of updating them directly.

timoschilling commented 8 years ago

Maybe we pick up the idea of Cell::Rails::Helper:: in cells-rails

apotonick commented 8 years ago

I can see your pain here, @hazah, the way we force users to include their helpers manually is ... awkward. It's a result of Rails' helper system, I was hoping this will put pressure on the core team to revisit this ugly part of Rails.

We could make your work part of cells-rails, as an optional feature (as @timoschilling suggested). Anyway, I still don't understand the Collection code - how's that related to the helpers? Thanks for your work! :beers:

timoschilling commented 8 years ago

Anyway, I still don't understand the Collection code - how's that related to the helpers?

@apotonick It is not related to the helpers, like @hazah says: https://github.com/apotonick/cells/pull/379/files#r53317187

hazah commented 8 years ago

@apotonick Yes, I've been following your distaste for helpers for a few years now. In general, I agree with you.

I would more than love to contribute to cells-rails if that's the direction things are going (I didn't find the repo).

As for the Collection code. It was late last night and I missed taking it out of the commit. The issue I have there is that cell collections have a different interface than regular cells (ie, they render immediately, while you still have to call #call on non collection cells). That made cell ignostic code more complicated.

apotonick commented 8 years ago

@hazah I added the repo here: https://github.com/trailblazer/cells-rails

As per the collections, I'm pretty sure @timoschilling had a PR here... Timo, bring it on.

hazah commented 8 years ago

@apotonick Awesome. Forked. Hopes are high that this can be actually figured out.

hazah commented 8 years ago

@apotonick, Sorry to highjack this PR. This is related but not really on the same topic.

Also, sorry to beat the dead horse that is truly dead. I wanted to mention that I ended up going in a different direction, and started working on a different variation on the "View Component" theme using the MVP pattern instead of MVC. I assure you that I am not attempting to repeat the architectures you've described. I don't know if that matters, but maybe you may find this approach more palatable -- its main distinction from MVC is that the entry point into these objects is the view, not the controller (which now controls only the view's presentation state and is called "presenter").

I hope that given similarities some ideas can be exchanged. I certainly want to see a proper HMVC implementation alive and well and may tackle that as well on my own. The two patterns can work side by side.

Repo is here: https://github.com/hazah/activeview

The dummy application is a small proof of concept.

apotonick commented 8 years ago

I read your repo @hazah but I'm struggling to see the difference to Cells? The naming (to me) is completely irrelevant, I don't even know what the official meaning for view model is haha. Is it only the wording in Cells that made you create this?

hazah commented 8 years ago

@apotonick, yes I understand that this is a rather nuanced distinction. I will attempt to explain that this is a little more than just naming.

Both patterns have essentially the same division in responsibility, this is why it's hard to tell them apart. In essence, the main difference is the point of entry & and their modes of operation.

Lets take our beloved MVC: We start at the controller and we call an action. This controller has the potential to have many actions, each of which has the potential to direct you to one of several views. So, MVC is a pattern where we have one controller mapping its actions to many views.

Now for MVP: We start at the view and we give it commands. The view itself only records the parameters of the command and triggers any number of actions on the presenter (what used to be the controller), which are responsible for getting the view ready for rendering. So, MVP is a pattern where we have one view mapping its commands to many actions.

The model layer is basically unchanged, except that since now you start from the view, the view objects themselves have an external interface that of a model (properties, callbacks, etc) -- so they can be called "view models" in the sense that they are modelling a view (the rendered data) through their properties. This also makes them something of a decorator, which leads to the "Presenter" pattern you normally hear about in Rails land where you just wrap a model and give it helper super powers.

I'm struggling to see the difference to Cells?

I think this is because Cells actually implements elements of both patterns at the same time, but without any separation of each case. For instance, the Cell itself is essentially a view object, but its API would have you believe that its actually a controller. The most common case for Cells seems to be a single action controller. When that is the case, the stack resembles the MVP approach, except for the fact that you don't have a clean command interface to the resulting Cell other than "render". So you're stuck implementing them as a procedure in your action (state as you call them) -- this action is performing the role of a presenter, setting the Cell up for rendering. But when you implement several actions in your Cell, it ceases to look like a view object, but is now a gateway to selecting one, and the Cell resembles a controller object.

The naming (to me) is completely irrelevant

I'm very much the same. I hope to have illustrated the actual distinction apart from the name (which is actually more of a convenience than anything else -- presenters are controllers, they are simply put to a slightly different task, which is to respond to commands on the view).

I don't even know what the official meaning for view model is haha.

It was invented by microsoft, so i wouldn't beat myself up over it ;). The technical distinction is that you have a view, a model, a view model (which is like a presenter but you execute commands on it directly instead of having the view forward them), and a binder that binds data properties from model to view for rendering and commands from view to view model for processing.

Is it only the wording in Cells that made you create this?

No, I've had this itch for years now. I have a very specific need of the implementation, and unfortunately nothing out there does it yet.

hazah commented 8 years ago

@apotonick If you get curious, you should now see a very clear difference between my repo and Cells. I had to figure out the interface. :beers: