-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change inflector to one that doesn't monkey patch #121
Comments
Thanks for the report, @cllns . Unfortunately I wasn't able to reproduce the effect you described in a new Hanami project (adding |
@zvkemp Thanks for the quick response. What a doozy! Took me a couple hours but I realized it was actually a bug with how a dependency of a dependency of Closing this since it's not a bug with |
@cllns My understanding is that requiring ActiveSupport shouldn't automatically do any monkey-patching. You have to explicitly require certain files for that to happen. Am I mistaken? |
@wagenet Yup! The issue is that a transitive dependency was requiring |
@cllns got it. Thanks for clarifying. |
I'd like to add Skylight to my Hanami project, but it depends on ActiveSupport. I see that someone else suggested removing this dependency but the proposal wasn't accepted.
In general, I'm really hesitant to pull ActiveSupport into my project, since it's very easy to accidentally introduce monkey-patched method calls. I really like Skylight so this is a huge bummer for me, but the risk of accidentally adding a monkey patch is too high for me.
@wagenet said:
I'm not sure where it's happening, but there's a core extension (monkey patch) being used, via ActiveSupport's Inflector.
Without
skylight
in my Gemfile,"dog".pluralize
rightly raises aNoMethodError
.With
skylight
in my Gemfile,"dog".pluralize
returns"dogs"
.A good, simple, and maintained, non-core extension library for inflections is
dry-inflector
.The text was updated successfully, but these errors were encountered: