I have been using stimulus in a rails 6 + turbolinks.+ ujs project for a couple months now. I really like it. I have been able to build out and showcase new versions of a customer workflow in days using the html + javascript sprinkles approach. One thing I have noticed, though, is in changing the way my frontend works, I have created many controller methods, and view partials that are no longer used. Has anyone else noticed this? Are there any good ways to find/manage these?
We do it old school. Sit and read the code. Note specifically for find methods that are not used. We read the code because it evolves. So we sit in small groups for 1-2 times a week for an hour and we read.
What I’ve found out is that you can clean up a lot in this way (although clean up is not the main goal of this ‘code reading’ as we call it). After you see a method it is sometimes Ctrl+D in sublime to see how it is used in the file and git grep to see if it is used anywhere. So we often find out logic that should be refactored or removed.
Update 1
The other thing that is important - I am not sure if you are talking about stimulus controller methods or rails controller methods. Having any number of methods in a rails controller is something we highly discourage. The controller should have no methods - only new, create, update,destroy, show, edit, index and because we use Cancancan there is a current_ability and {object}_params methods. No other methods in the controllers. We are not extreme about this, but we like the controllers with no other methods and we consider it a smell if a controller needs other methods.
Thanks for responding. That sounds like a very good approach. Make it a code review practice. I also agree with you and avoid non-standard index, new, edit, update, create, destroy methods in controllers. But some of these methods return partials used in stimulus ajax calls, exclusively. You have to be very careful as your codebase evolves to look for orphaned controller methods, and their partials, as they can easily build up over time as you change the architecture of your stimulus frontend and rails backend.
I am not sure I understand what this methods are doing and why? Why do you need to make stimulus ajax calls that are answered by small methods in the controller. Why are this not separate controllers? Could you give an example?
so, I have a single view where the user can manage a worksheet for a months requests, and individual requests for that month. The basic rails resources are worksheet > request. A worksheeet has many requests. I allow the user to list all the requests for the worksheet in the same view. They can then edit an individual request. This makes an ajax call to get the form for the request and inline replace the original item with the form. They can then submit the form, and stimulus handles the ajax:success or ajax:failure by inline replacing the form with either the item returned by create/update, or the form with errors. All ajax uses calls are to standard worksheet and request controller edit, update, create, destroy endpoints. the request endpoints just return partials.
Thanks. The request controller has an edit method that returns the form of editing, right? Which is the additional method that is not an new,show,edit,create,update,destroy?
Hey @dmlond! I’m late to the game here, but figure I’d piggy back on some of @thebravoman’s advice here.
These types of problems sound like an anti-pattern, pointing at issues with code organization rather than simply being the cost of using Stimulus. As @thebravoman pointed out, limiting your controller actions can help out quite a bit here. This is what DHH and Basecamp encourage (and here’s an article discussing it) and I can attest that it’s a far better approach.
That said, it’s not as simple as limiting your controller actions. It can take a bit of a mindset flip to break things out into more discrete pieces, often times thinking of controllers as representing functionality rather than resources. This approach will more likely lead to having orphaned controllers rather than orphaned methods, which are easier to spot.
As an example, my current app can list all of the users for account administrators. I can also list only the admin users. This is a completely separate controller, Users/AdminController vs. UsersController, despite the similarities. Users/AdminController actually does just re-use the UsersController view, though!
But… I don’t suspect that’s where you’re running into trouble. So let’s dig in a little deeper. On any of the users listings, you can perform various actions on users. For instance, suspend, make admin, or revoke admin. These are all separate controllers - not edit actions on UsersController or Users/AdminController. So we end up with controllers such as:
class UserSuspensionController < AdminController
def create
user = Current.company.users.find(params['id'])
user.suspend!
flash.notice = "#{user.name_or_email} has been suspended"
redirect_back fallback_location: account_path
end
end
And that’s it. That’s the entire controller. If users can no longer be suspended, this controller will stick out much more than if this functionality was buried in the UsersController. Alongside this controller is the UserReactivationController, which is simply this in reverse.
Another example would be making messages in my app private. That’s done by a PrivatizeMessageController, not the edit action of a MessagesController.
I’ll also point out that in the above example, I’m not rendering a partial but instead just allowing the referring page to be entirely re-rendered. I certainly have AJAXy operations as well, but when using Turbolinks this can be a great approach, too.
That said, when you are rendering partials, this approach allows you to just set the layout to none at the controller level and render a normal view as a partial, effectively, with the same code organization that comes along with normal views. In the case of the UserSuspensionController above, there are zero views or partials that go along with it, which makes it super easy to refactor or remove. However, if it did have any views, they would be in app/views/user_suspension/… meaning I can just delete that whole directory were I to delete the controller.
I hope I’m touching on the right points here and this advice is helpful. Let me know either way and we’ll keep trying to help!
Cheers!
1 Like
Thanks to @thebravoman and @welearnednothing for giving me encouragement to try some major refactoring on the Rails side in a couple apps. While name spacing controllers etc have been around for a long time, I never tried them. The article on how basecamp does things, while informative, didn’t help me with my first attempt. Even searching for rails namespaced controller didn’t point to a Rails application that I could see the overall structure of the application.
After of few day of learning I finally got a simple example working. The biggest problem was the namespace routes. Didn’t know that routes order matters!. My simple example was just to factor out a couple action calls from my Accounts controller. The CRUD show action was not really a show view of the account, but a recent Ledger view that shows the recent Entries(transactions). The other was just a filter on the index action. My first attempt and adding the routes failed.
resources :accounts
namespace :accounts do
resources :bank, only: :index
resources :ledger, only: :show
end
The ledger action worked fine with accounts/ledger/5 but the filter showing only the BANK accounts accounts/bank failed with 'id BANK not found. Putting resources :accounts` after the namespace block got the filter to work.
namespace :accounts do
resources :bank, only: :index
resources :ledger, only: :show
end
resources :accounts
At least I have a idea on how to do this, but I have more learning to do.
Again, thanks for the ideas.