Need a way to add new converters

[Suor]

I am trying to add support for JSON fields here - https://github.com/Suor/sql-bricks-postgres/pull/3. But looks like I can only do it overwriting sql.convert(), e.g. messing with original namespace.

Any ideas?

[prust]

@Suor: Yeah, sql.convert()/sql.conversions is not very extensible the way it is:

  sql.conversions = {
    'String': function(str) { return "'" + str.replace(/'/g, "''") + "'"; },
    'Null': function() { return 'null'; },
    'Undefined': function() { return 'null'; },
    'Number': function(num) { return num.toString(); },
    'Boolean': function(bool) { return bool.toString().toUpperCase(); },
    'Date': function(dt) { return "TIMESTAMP WITH TIME ZONE '" + dt.toISOString().replace('T', ' ').replace('Z', '+00:00') + "'"; },
    'Array': function(arr) { return '{' + arr.map(sql.convert).join(', ') + '}'; }
  };

I think all it would take to make it more extensible would be this (not sure the .bind(_) is necessary):

  sql.conversions = [
    [_.isString.bind(_), function(str) { return "'" + str.replace(/'/g, "''") + "'"; }],
    [_.isNull.bind(_), function() { return 'null'; }],
    [_.isUndefined.bind(_), function() { return 'null'; }],
    [_.isNumber.bind(_), function(num) { return num.toString(); }],
    [_.isBoolean.bind(_), function(bool) { return bool.toString().toUpperCase(); }],
    [_.isDate.bind(_), function(dt) { return "TIMESTAMP WITH TIME ZONE '" + dt.toISOString().replace('T', ' ').replace('Z', '+00:00') + "'"; }],
    [_.isArray.bind(_), function(arr) { return '{' + arr.map(sql.convert).join(', ') + '}'; }]
  ];

Then to extend it you could just do:

sql.conversions.push([
  function(val) { return _.isObject(val) && !_.isArray(val) && !_.isArguments(val); },
  function(val) { return sql.convert(JSON.stringify(val)); }
]);

I'll go ahead & make those changes...

[Suor]

But if I change sql.conversions I will change behavior of original sql-bricks, not my extension.

[prust]

@Suor: Oh, I see. The only way I can think to support this is by making sql an instance of a class:

// inside sql-bricks.js:
module.exports = new SqlBricks();

// calling code would remain unchanged
var sql = require('sql-bricks');

sql._extension() would then return an instance of the subclass:

  sql._extension = function () {
    var SubClass = subclass(SqlBricks);
    var ext = new SubClass();

    // ... other logic already in sql._extension()

    return ext;
  }

Then internally sql-bricks would call this.convert() instead of sql.convert() and if you override convert() in your subclass, the override will be called instead.

What do you think?

[Suor]

That might work, you'll however need to change to this. everywhere. Also, ._extension() should probably return class.

[prust]

For the record: Suor was able to hack around this in https://github.com/Suor/sql-bricks-postgres/commit/c5a6975c98df8044d2, but really we should fix this so that he doesn't have to.