rockdai / sql-bricks

Transparent, Schemaless SQL Generation
http://csnw.github.io/sql-bricks
MIT License
213 stars 25 forks source link

Replace custom templating with ES2015 template literals #100

Closed prust closed 6 years ago

prust commented 6 years ago

The mini-templating language I wrote for sql-bricks enabled short clause definitions, but it added a lot of complexity, making it more difficult for developers new to the project to follow the code.

Since ES2015 introduced template literals, we can use those instead. It takes more lines to define SQL clauses, but the flow of control is easier to follow. Also, it enables us to remove the templating language implementation, which results in a net reduction in lines of code.

This is a breaking change, so I'll probably release it as a 3.0.0-beta.1 version. The following dependent libraries will need to be updated:

This will remove support for Node 10.x and 12.x, as well as IE11 support. I'm not sure why anyone would be using this code on IE, since it never supported WebSQL, but if necessary, it should be possible to transpile the code into older ECMAScript that supports IE and older versions of Node.

prust commented 6 years ago

@Suor: What do you think of this change?

Suor commented 6 years ago

It will probably make it simpler for newcomers, and if you feel that it will simplify it for you too then you should go with it. I am fine with both ways.

Another thing you might want to stop writing methods to statement in .defineClause(). Like replace:

this.prototype[clause_id + 'ToString'] = templ_fn;
// ...
clauses.splice(index, 0, clause_id);

with

clauses.splice(index, 0, templ_fn); // Better name is render_fn()

Then ._toString() could be simplified too:

Statement.prototype._toString = function(opts) {
    return _(this.clauses).map(clause => clause.call(this, opts)).compact().join(' ');
};

Also, you can give more use to arrow functions, you do many .bind(this) now, which is not needed with them.

prust commented 6 years ago

@Suor: Thanks! I followed your suggestion in e481b6f & cbe4591, but I did have to add the clause_id as a property of the render_fn so that before/after still work.

you can give more use to arrow functions

I'll consider this -- I haven't been too enamored with arrow functions so far, but perhaps I haven't given them enough of a fair shot yet.

FYI: I'm going to merge this and publish as 3.0.0-beta.1, but there's no rush to support it since I want to include a few other deep changes to 3.0 before release.