Pundit: Module `Pundit` is too big

Created on 2 Jun 2018  路  2Comments  路  Source: varvet/pundit

I got a real problem when started to use Pundit. I have my own method permitted_attributes which of course conflicts with Pundit same method. I think the included Pundit module should be as small as possible, it should contain only public methods, the rest may go to other classes to avoid polluting ApplicationController. I solved the problem by renaming my method, so this is not urgent. Anyway it would be great to refactor.

enhancement

Most helpful comment

At least pundit should expose protected method pundit_permitted_attributes instead of permitted_attributes for the consistency.

All 2 comments

At least pundit should expose protected method pundit_permitted_attributes instead of permitted_attributes for the consistency.

I see two separate issues when it comes to Pundit pollution.

Issue 1: Including top level module

Including top level Pundit module has huge side effects. It not only includes methods defined in Pundit module, but also adds all internal classes and modules to the scope. And not only the modules that are defined in pundit.rb but literally everything! In your controller you can access constants like VERSION, SUFFIX or even RSpec from lib/pundit/rspec.rb.

Solution for that is extracting only the public interface to a module inside Pundit global module, e.g. Pundit::ControllerExtension. In order to prevent compatibility change Pundit global module could still work the old way, by including new module (ControllerExtension) in self.included() with a deprecation message.

Fixing it should be rather simple and backward compatible. PR: https://github.com/varvet/pundit/pull/621

Issue 2: Unexpected methods pollution

Pundit "pollutes" both class and instance scopes with some methods that are not prefix with pundit_. As it's been noted in this Issue it might force some users to rename their own methods and lead to unexpected behaviours, especially for junior devs who may be completely unaware of that.

I'd suggest at least prepending all the methods with pundit_ prefix to make that clear.

That would require introducing breaking changes to the interface. Before starting to work on it I would love to have a clear consensus from the gem owner(s) about this breaking change.

Methods pollution in general

Ideally Pundit in general could avoid method pollution. This could be achieved by hiding all the methods behind a single object, accessible using only one method, e.g. pundit.

This has its big downsides and probably is considered a non-Rails way. I would personally avoid that since you can always use Pundit without including it to your controller.

Was this page helpful?
0 / 5 - 0 ratings