search

graphiti-api / graphiti-rails

MIT License
53 stars 28 forks source link

Fix rake task helpers polluting global namespace #91

Open hibachrach opened 1 year ago

hibachrach commented 1 year ago

Before, the session method (for example) would be redefined in the context of an RSpec request spec because the default definee in the block within the namespace call would be main, the global object.

Additional context about default definees: https://blog.yugui.jp/entry/846

jasonkarns commented 1 year ago

Another option is to define the methods in an external file/module, and only require them from within the task defs. That way they're only included when those tasks are actually run.

hibachrach commented 1 year ago

Would you prefer that option? Willing to edit the PR if that's the case.

jasonkarns commented 1 year ago

@hibachrach I'm not one of the maintainers, so I can't speak for them or the project. However, moving the module(s) into an external file that is explicitly (and only) required when needed is the approach I use and recommend on my teams. As it stands in this PR, the Graphiti::Rails::RakeHelpers modules are all being defined whether those tasks are invoked or not. (The namespace body is evaluated when rake loads the file, as that is how rake knows what tasks exist.)

What's more, then you can include Graphiti::Rails::RakeHelpers in the task body and use the methods inline (unqualified) as they were before.