[RFC] Remove dependency on view_context

[n-rodriguez]

Hi there!

In order to have a cleaner API for ajax-datatables-rails and in a long term view for this gem, I'd like to remove the view_context dependency by directly injecting the params hash.

It would have some advantages :

• start to reduce coupling with Rails (there's still room for improvements but it would be a start)
• ajax-datatables-rails would be compatible with Rails API which doesn't have view_context (Using in API only mode? #240)
• it would make tests easier for users (you have only one hash to pass to build and test a datatable, don't need Capybara tests anymore)
• the view_context is a quite big object to pass around, passing a hash would be lighter
• it encourages developers to have a clean object design

But it also have some drawbacks :

• it changes the API :

# before
  respond_to do |format|
    format.json { render json: UserDatatable.new(view_context) }
  end

# after
  respond_to do |format|
    format.json { render json: UserDatatable.new(params) }
  end

• you cannot delegate methods on the view_context from within the datatable. (as in https://github.com/jbox-web/ajax-datatables-rails#using-view-helpers), but you can use Draper which is IMHO a much cleaner implementation for this.
• it adds a dependency on Draper for users (it won't be part of ajax-datatable-rails)

If you really want to inject the view_context as before you can still use the options hash :

# Controller
  respond_to do |format|
    format.json { render json: UserDatatable.new(params, view_context: view_context) }
  end

# Datatable
class ApplicationDatatable < AjaxDatatablesRails::Base
  extend Forwardable
  attr_reader :view
  def initialize(params, opts = {})
    @view = opts[:view_context]
    super
  end
end

class MyCustomDatatable < ApplicationDatatable
end

I'm testing this idea on a real world application (45 datatables) with this branch : https://github.com/jbox-web/ajax-datatables-rails/tree/feat/rails-api and so far it works very well :) (I made the migration on Draper before)

Tests are passing : https://travis-ci.org/jbox-web/ajax-datatables-rails/builds/375203557 🎉

The idea was a lot inspired by Trailblazer which I use in my app 👍

What do you think? It would be for a 0.5 release.

[n-rodriguez]

cc @ajahongir

[Nittarab]

Oh, this is an important change... I have a question: are you sure that Draper are still updated? Don't see a lot of activity in that repo.
Btw, if I include an Helper where I have one method that use the link_to method, this will not work right?

[antillas21]

What do you think? It would be for a 0.5 release.

I believe going this way, you should bump to 1.0 release, as this is a major/breaking change.
I like the idea on removing view_context dependency.

as for:

are you sure that Draper are still updated?

You don't need a project to have (code) activity every so often, IMHO, you only need a stable project.

These are (literally) my two cents 🙂

[ajahongir]

ajax-datatables-rails would be compatible with Rails API which doesn't have view_context (#240)

It sounds good and totally agree with @antillas21 to bump up 1.0 release

[n-rodriguez]

Hi!

Oh, this is an important change... I have a question: are you sure that Draper are still updated? Don't see a lot of activity in that repo.

Actually Draper is an optional dependency and will not be installed by ajax-datatable-rails.
Draper is just a convenient way of formatting data before rendering them in JSON.
You can also use the way I proposed above.
I also agree with @antillas21 :

You don't need a project to have (code) activity every so often, IMHO, you only need a stable project.

Also

I believe going this way, you should bump to 1.0 release, as this is a major/breaking change.

I agree that at some point we should bump to 1.0 but I'd love to fix #228 and some other issues before releasing a 1.0. The goal is to have a stable API for the 1.0 release. I don't know yet how solving #228 would impact the API. Thus the 0.5 release.
As far as I remember this change doesn't break semver since we're still in 0.x.x.
What do you think?

[n-rodriguez]

And thanks everybody for your answers :)

[n-rodriguez]

Hi there! After a lof of thinking on how solve this #228 I came with some solutions.
First of all we should not extend AjaxDatatablesRails::Base class at runtime as it breaks Ruby method cache : https://github.com/jbox-web/ajax-datatables-rails/blob/master/lib/ajax-datatables-rails/base.rb#L120. Not very good for performances.

The ORM module should be included in the AjaxDatatablesRails::Base class or subclass.

To solve the "extend at runtime" issue we can :

• let the user include the AjaxDatatablesRails::ORM::ActiveRecord module in every single class (not very friendly)
• add a new class AjaxDatatablesRails::ActiveRecord that would inherit from AjaxDatatablesRails::Base and users would inherit from AjaxDatatablesRails::ActiveRecord (a bit more friendly and it let room for AjaxDatatablesRails::Mongoid)

It also removes the need to configure the ORM (a bit of convention over configuration).

Notes: I've already thought of dependency injection but it's not doable due to the current implementation : https://github.com/jbox-web/ajax-datatables-rails/blob/master/lib/ajax-datatables-rails/orm/active_record.rb#L27.
The ORM module knows about the datatable object and you can't inject it as it creates a chicken and egg issue :

def initialize(params, opts = {})
...
@orm = opts.fetch(:orm, ActiveRecord.new(self))
end

but how do you do :

orm = Mongo.new(dt)
dt = PostDatatable.new(params, orm: orm)

Or we can just inject the class name and call .new on it... but it's not very performance oriented. Instantiating a new ORM object that could be included in a class...

On the other side it would make cleaner "interfaces" and responsabilities between objects...

I tried both :

• The "class" solution is here https://github.com/jbox-web/ajax-datatables-rails/tree/feat/ar_class
• The "injection" solution breaks a lots of tests and needs some rewrite so I have no branch for it yet...

[n-rodriguez]

Finally the "injection" solution doesn't solve the very first issue : "Allow again overriding filter_records, sort_records" so it's a no go :)

[n-rodriguez]

Hi there! Any thoughts on the last proposal? (AjaxDatatablesRails::ActiveRecord vs AjaxDatatablesRails::Base).
With the view_context change it's 2 good reasons to bump to v1.0.0 👍

[n-rodriguez]

Release 1.0.0 is out! https://github.com/jbox-web/ajax-datatables-rails/releases/tag/v1.0.0