hashrocket / decent_exposure

A helper for creating declarative interfaces in controllers
MIT License
1.81k stars 107 forks source link

feat(loader): enable lazy loading on specific actions #198

Closed rainerborene closed 1 year ago

rainerborene commented 3 years ago

One of the drawbacks of expose! is that it run on every controller action. The idea behind this pull request is to allow developers to eliminate lazy loadiness on specific actions without the weirdness of before_action :thing, except: :sort. So, instead you can use before_action :load_thing, except: :sort. More clarity and better naming conventions. It will also really eliminate lazy load for ActiveRecord collections by calling load before.

mattpolito commented 3 years ago

Hello @rainerborene, thanks for the PR. I'm a bit confused by this request though as exposures are lazy already by using expose.

They are only forced when you expose!.

rainerborene commented 3 years ago

@mattpolito Sorry, I didn't make myself clear. This feature improves expose! by allowing you to eliminate lazy loadiness on specific controller actions. expose! runs on every controller action by default, and that's the problem. It's worth mentioning that expose! will not eliminate lazy load of ActiveRecord collections unless you call load explicity.

rainerborene commented 3 years ago

@mattpolito Do you think this pull request could be simplified to this instead?

def expose!(name, *args, **options, &block)
  before_options = options.extract!(:only, :except)
  expose name, *args, **options, &block
  before_action name, before_options
end
mattpolito commented 3 years ago

That would definitely be clearer as far as code goes, however, could you describe a scenario where you would need this? We've used DE for many many years and haven't run into a scenario where this has come up. I'm curious where you're running into this.

rainerborene commented 3 years ago

@mattpolito Fair enough. Say you have comments collection that could trigger an exception based on current user permission, and also, you want to run it only for index action. I think the obvious and most common scenario is when you don't have restful routes. You don't want to expose! comments on every controller action, right?

mattpolito commented 3 years ago

Understood. Is there a reason you don't want to use expose then? Based on your description it would seem like that's what you're looking for.

expose! would only be for when you need the exposure loaded ahead of time. Honestly I've never once needed to use it.