Closed Suor closed 9 years ago
@Suor: right -- this is why I originally proposed an additional function call, the ()
at the end of require('sql-bricks')()
. But I think you're right: that API was unintuitive.
With the way node caches modules, we can't make require('sql-bricks')
return something different inside sql-bricks-postgres than it does in the end-user's program. The only alternative I can think of is for sql-bricks-postgres and other extensions to use a different API than normal users, something like require('sql-bricks').extend(...)
, which could return a new, separate, namespace that could be extended...
Does this sound like a good approach? Can you think of other options?
There is no connection of sql-bricks-postgres
and friends require interface and dirtying. We just need to stop writing to shared sql
namespace and its constructors.
An extension can export prototype descendant instead:
var pgsql = Object.create(sql);
module.exports = pgsql;
And maybe inherit constructors with sql.inherits
:
function Select() {
if (!(this instanceof Select))
this = Object.create(Select.prototype);
Select.super_.apply(this, arguments);
return this;
}
pgsql.select = sql.inherits(Select, sql.Select);
@Suor:
var pgsql = Object.create(sql);
I think I see. I don't have much experience with Object.create()
, but my understanding is that this will set pgsql
's prototype to the sql
namespace -- basically using prototype chaining to automatically delegate calls down, but without requiring the user to "instantiate" anything (the user wouldn't need to do var pgsql = new require('sql-bricks-postgres')()
). Cool idea, I hadn't thought of that.
I like this direction -- are there any changes I would need to make to the underlying sql-bricks library, or is this just something that would be implemented in extensions (sql-bricks-postgres and sql-bricks-sqlite)?
I'll try updating sql-bricks-postgres
this way. Most probably this could be done with no changes to sql-bricks
itself. However, some things could be shared to make creating extensions easier. I'll get back when I have something to show.
I managed to undirty my extension - https://github.com/Suor/sql-bricks-postgres/commit/bf6eff88277de1fc30c796dba2bf2cbce2ef54c8. It required some hassle to make subclasses, which have safe constructors and pass all their arguments to base.
Object.create()
didn't work cause it's not really mixable with your class-like approach. Things could've been smooth if you used prototypal inheritance in sql-bricks
too.
Regarding moving anything to sql-bricks
, it's your call. However, makeSubclass()
could be moved (optionally stripped from defineClause
specific). Or you can move in the whole thing, so that extension would just write:
var pgsql = sql._extension();
pgsql.select.prototype.smth = ...
...
This, however, will clone everything even if it's not needed.
I also needed to copy everything that's not on prototype, but namespace itself - https://github.com/Suor/sql-bricks-postgres/commit/ad302520ed40c69bf48ce2a07974cea5aa0d791b
I read through both commits and they look perfect to me; good work @Suor!
Or you can move in the whole thing
This makes the most sense to me -- do you want to submit this as a pull request?
Ok. Tomorrow.
For the record, this was addressed in #55, #56 and finally closed in #57.
The way extensions are implemented they extend core namespace and classes inplace. So if anyone uses shared
sql-bricks.js
, like in browser ornpm dedupe
environment, things could clash. See howsql
is mutated here: