Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make it available as a commonjs module #12

Closed
wants to merge 1 commit into from
Closed

make it available as a commonjs module #12

wants to merge 1 commit into from

Conversation

mfferreira
Copy link

No description provided.

@nicjansma
Copy link
Contributor

I have a similar patch I'm working with. Instead of looking for exports at the end, I wrap the var joli statement in an a create function that returns joli.

The benefit of doing it this way, is you can use joli for two separate connections/databases.

The downside is, this forces Joli to be a CommonJS module.

Any thoughts on how to have it flexable (checking for exports, so it isn't required to be used as a CommonJS module) yet keeping the var joli in a closure to allow for separate joli instances?

e.g., my code is:

exports.create = function() {

    // START of original joli.js
    var joli = {
    ...
    }
    return joli;
};

@mfferreira
Copy link
Author

I think both ideas can join like so:

if (exports) {
    exports.create = function() {
        return joli;
    };
}

@nicjansma
Copy link
Contributor

Unfortunately, that doesn't allow you to have two separate joli instances, since they're created off the same var.

@nicjansma
Copy link
Contributor

Maybe:

var joliCreator = function() {
    var joli = { ... };
    return joli;
};
var joli = joliCreator();
if (exports) {
    exports.joli = joliCreator;
}

Then, you can get a new Joli instance by calling require('joli').joli(), and yet joli is still a global var if you're not using CommonJS.

@mfferreira
Copy link
Author

Oh right.
Well u take some parts out from that:

var joli = function() {
    this.each = function() { 
        ...
    };
    this.extend = function() { 
        ...
    };
    return this;
};
if (exports) {
    exports.joli = joli;
}

Its about the same thing, just a different approach.

@nicjansma
Copy link
Contributor

But that doesn't give you parity for existing apps, since joli is now a function that needs to be executed first, correct?

For example, if someone isn't using CommonJS, they would now have to use:

var myjoli = joli();

That's why I suggested a separate function joliCreator(), then still setting joli as an object, and exporting that, you get parity for non-CommonJS cases, as well as the exports for CommonJS (which would be slightly differently called like var joli = exports('joli').joli(), but that seems reasonable)

@mfferreira
Copy link
Author

Correct. Maybe just one minor change then:

var joliCreator = function() {
    var joli = { ... };
    return joli;
};
var joli = joliCreator();
if (exports) {
    exports.joli = joliCreator();
}

@nicjansma
Copy link
Contributor

Cool, that works great.

@xavierlacot
Copy link
Owner

Hi guys,

I have just pushed a variation of your common work :

var joli = {
    // all the previous stuff
};

if (typeof exports === 'object' && exports) {
    exports = function() {
        var api = {
            connect: function(database) {

                if (database) {
                    joli.connection = new joli.Connection(database);
                }

                return joli;
            }
        };

        return api;
    }();
}

This allows to either load the library the way we did before:

Titanium.include('joli.js');
joli.connection = new joli.Connection('your_database_name');

or the new way:

var joli = require('path/to/joli').connect('your_database_name');

Thanks both for the contribution!

@nicjansma
Copy link
Contributor

Thanks Xavier. The one change that isn't captured in your update is the ability to use Joli on two separate databases, because var joli = {} is initialized at the root of the module, instead of in a function closure: function() { var joli = ...; return joli}.

The parsing of CommonJS modules may be cached, so the root-level var joli is only ever initialzied once. With your change, the second call to .connect() will overwrite the settings of previous calls.

For example:

var joli1 = require('path/to/joli').connect('db1');
var joli2 = require('path/to/joli').connect('db2'); // joli1 now points to db2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants