GeppettoJS / backbone.geppetto

Bring your Backbone applications to life with an event-driven Command framework.
http://geppettojs.github.com/backbone.geppetto/
MIT License
203 stars 28 forks source link

impossible to wire falsy values #80

Open mmikeyy opened 9 years ago

mmikeyy commented 9 years ago

Hey! I'm shortly reemerging between two projects. Since my last presence here, I've been using Geppetto full time. 7 days a week! It's now second nature for me and I appreciate it more than ever.

I'd just like to report one issue. There is a problem with the wiring of values. The documentation says:

context.wireValue("answerToLifeTheUniverseAndEverything", 42);

but nothing lets the reader suspect that the answer to life, the universe and everything could never be zero (which would be a more probable answer! :smile: )

The reason, as far as I can tell, is as follows. It starts with the wireValue method, which labels the value as a singleton:

        wireValue: function(key, useValue) {
            this._mappings[key] = {
                clazz: null,
                object: useValue,
                type: TYPES.SINGLETON
            };
            return this;
        },

The value to be wired (useValue) is stored as the mapping's 'object' property.

Later, when the time comes to retrieve the value, the code looks at the 'object' property and if it evaluates to false, it decides that there must be a class stored in the clazz property that needs to be instantiated:

        _retrieveFromCacheOrCreate: function(key, overrideRules) {
            var output;
            if (this._mappings.hasOwnProperty(key)) {
                var config = this._mappings[key];
                if (!overrideRules && config.type === TYPES.SINGLETON) {
                    if (!config.object) { // <<<<< problem is here
                        config.object = this._createAndSetupInstance(config);
                    }
                    output = config.object;

But for a wired value, there is no clazz and this fails to be noticed.

As mentioned above, the problem is with this line:

          if (!config.object) {

... which causes this._createAndSetupInstance to be called, which will instantiate mapping.clazz, which is wrongly assumed to be defined.

The simplest solution I can think of is to just replace the line

          if (!config.object) {

with the following:

          if (!config.object && config.clazz) {

With the code changed this way, one can define a singleton as any falsy value and everything should work as expected.

And then context.wireValue("answerToLifeTheUniverseAndEverything", null); will be perfectly valid, and Geppetto will be less opinionated! :wink:

creynders commented 9 years ago

Quite right! Feel free to create PR (or I will if you don't have the time) with the patch-branch as a target.