Skip to content
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

Remove ActiveSupport? #33

Closed
mwpastore opened this issue Nov 13, 2015 · 7 comments
Closed

Remove ActiveSupport? #33

mwpastore opened this issue Nov 13, 2015 · 7 comments

Comments

@mwpastore
Copy link
Contributor

Since most of us are probably using this particular gem specifically to avoid pulling half of Rails into our non-Rails applications, what would it take to remove the dependency on ActiveSupport? Looks like it's being used primarily for inflection.

@fotinakis
Copy link
Owner

I think this is the main or only place it's used: object.class.name.demodulize.tableize.dasherize — which is actually a significant amount of logic underneath. If you can find a different gem that provides just that limited functionality in a backwards-compatible way, I'd be willing to look at changing things—but honestly, from what I can tell most users of this gem are actually using it in a Rails environment, so I think keeping the ActiveSupport dependency is the best way. I don't think it's "half of Rails", but I don't have a good sense for how big the gem actually is — actually, according to RubyGems it's 4.2.5 - November 12, 2015 (323 KB), which I'm personally fine with.

@mwpastore
Copy link
Contributor Author

Thanks Mike. I didn't mean to imply that JAS specifically was pulling in half of Rails, just that in general if you pull one thread of it you tend to get the whole ball of mud. ;-) Actually, I think it's even better than that because you're only require'ing active_support/inflectors, which is only about 29,171 bytes (maybe a bit more after following all the subsequent require's). I may be imagining dragons here.

@mwpastore
Copy link
Contributor Author

If you're interested in pursuing this further, I was able to narrow the scope even further by require'ing active_support/inflector/methods instead of active_support/inflector and the tests pass. Either way, I'm satisfied. Thanks again.

diff --git a/lib/jsonapi-serializers/serializer.rb b/lib/jsonapi-serializers/serializer.rb
index f7117f0..d23ad6e 100644
--- a/lib/jsonapi-serializers/serializer.rb
+++ b/lib/jsonapi-serializers/serializer.rb
@@ -1,5 +1,5 @@
 require 'set'
-require 'active_support/inflector'
+require 'active_support/inflector/methods'

 module JSONAPI
   module Serializer

@fotinakis
Copy link
Owner

Sounds good. :) I think this is probably fine for now. There are some other major performance things inside this gem that I want to address at some point, but those are mostly algorithmic and not dependency related.

They're also working on JSON-API support over in ActiveModelSerializers if you're interested: rails-api/active_model_serializers#1235 — I've been lightly following it, looking to see where it lands when stable and see if I should just recommend that people use that instead.

@mwpastore
Copy link
Contributor Author

Nooooo, I love this Gem! Unless AMS adds support for Sequel ORM, that's not going to work for me. 😦

@fotinakis
Copy link
Owner

Hah, well that's good to know. :)

@mwpastore
Copy link
Contributor Author

The tests pass, but serialization fails in my app, so ignore that patch. It don't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants