Make array like classes configurable #6
Conversation
… make-array-like-classes-configurable
| end | ||
|
|
||
| def array_like?(object) | ||
| object.is_a?(Array) || active_record_relation?(object) | ||
| array_like_classes.any? do |klass| |
There was a problem hiding this comment.
I got rid of the array_like_class? helper method since all this method did was call that method, but if you prefer it for readability I can move this logic back to that method and just call that method from here.
| def array_like_classes | ||
| [ | ||
| Array, | ||
| defined?(ActiveRecord::Relation) && ActiveRecord::Relation, |
There was a problem hiding this comment.
if it's not defined this becomes nil and the .compact at the end gets rid of it.
There was a problem hiding this comment.
since this is config and won't change at runtime, should we consider overriding the default getter or setter and inject this behaviour there? that way, it'll be memoized and we won't create/compute this array at runtime for each render call.
PS: sprinkling a memoization logic here might look ugly since this is a helper class.
|
|
||
| context 'and is not an instance of a configured array-like class' do | ||
| it 'should raise an error' do | ||
| expect { blueprint.render(obj) }.to raise_error(NoMethodError) |
There was a problem hiding this comment.
there's not really anything else to assert here. We already have tests for that cover a hash being handled properly, and hashes are "array-like" in that they are Enumerable, so here I just mimicked what would happen if you pass it a collection for which you didn't pass the class of to array_like_classes.
|
@ritikesh once this gets merged in can we get a new version of the gem pushed? |
| def array_like_classes | ||
| [ | ||
| Array, | ||
| defined?(ActiveRecord::Relation) && ActiveRecord::Relation, |
There was a problem hiding this comment.
since this is config and won't change at runtime, should we consider overriding the default getter or setter and inject this behaviour there? that way, it'll be memoized and we won't create/compute this array at runtime for each render call.
PS: sprinkling a memoization logic here might look ugly since this is a helper class.
README.md
Outdated
| You can also configure other classes to be treated like collections. For example, if you are using Mongoid, you can configure it to treat `Mongoid::Criteria` objects as collections: | ||
|
|
||
| ```ruby | ||
| Blueprinter.configure do |config| | ||
| config.array_like_classes = [Mongoid::Criteria] | ||
| end | ||
| ``` | ||
|
|
||
| Or if you wanted it to treat the `Set` class as a collection: | ||
|
|
||
| ```ruby | ||
| Blueprinter.configure do |config| | ||
| config.array_like_classes = [Set] | ||
| end |
There was a problem hiding this comment.
can you pls create a changelog entry and bump the patch version?
…dnestor/blueprinter into make-array-like-classes-configurable
|
released version 1.1.0 |
At @RaiseMe we have a large Rails project that uses
Mongoidwith mongo instead ofActiveRecordwith a relational database. As such our collections return instances ofMongoid::Criteriainstead ofActiveRecord::Relation, for example:As a result when we use
Blueprinterwe have to call.to_aon our collections, such asUserBlueprint.render(User.all.to_a)instead of just doingUserBlueprint.render(User.all). This is both inconvenient, as well as having a performance impact since we can't utilize cursors when iterating over a collection. Calling#to_acauses it to load all the matching documents into memory before iterating over them.Originally I attempted to fix this by making it treat anything
Enumerableas an array (#2), but this was too general and could cause potential issues. Next I was going to make it just treatMongoid::Criteriaas arrays (see diff here), but while I was putting in that PR I realized it would make more sense to just make this configurable. Making it configurable will have zero impact on users currently using the gem, allow users to customize it to their own needs, and doesn't require adding theMongoidgem to the dev dependencies for theBlueprintergem, as well as alleviating the testing overhead there (I ended up stubbing outMongoid::Criteriaon my attempt for that to avoid requiring havingmongorunning, but still wasn't a great approach).This PR is a much more minimal approach than those other two and hopefully it will be acceptable.
Let me know if you have any questions!