Flotype / now

NowJS makes it easy to build real-time web apps using JavaScript
http://www.nowjs.com
MIT License
1.91k stars 175 forks source link

Misbehavior of nowUtil.hasProperty #119

Closed Nican closed 13 years ago

Nican commented 13 years ago

I narrow a bug down from one merging a client down to a group.

I changed the nowUtil.mergeScopescode to:

  mergeScopes: function(current, incoming) {
    console.log( "incoming: ",  current, incoming );

    for(var prop in incoming){
      if(nowUtil.hasProperty(incoming, prop)) {
        console.log("", prop );
        if(incoming[prop] && typeof incoming[prop] === "object"){
          if(!nowUtil.hasProperty(current, prop)){
            console.log("", "does not have prop");
            if(nowUtil.isArray(incoming[prop])) {
              current[prop] = [];
            } else {
              current[prop] = {};
            }
          }
          nowUtil.mergeScopes(current[prop], incoming[prop]);
        } else {
          current[prop] = incoming[prop];
        }
      }
    }
  },

And got the following output:

incoming:  {} { test: [ 5 ] }
 test
incoming:  undefined [ 5 ]
 0
TypeError: Cannot set property '0' of undefined
    at Object.mergeScopes (/srv/irc/node_modules/now/lib/nowUtil.js:294:25)
    at Object.mergeScopes (/srv/irc/node_modules/now/lib/nowUtil.js:292:19)
    at ClientGroup.addUser (/srv/irc/node_modules/now/lib/clientGroup.js:97:15)

I was expecting "does not have prop" to be called so that the "test" array is created in the current object, but it somehow seems to return false.

Changing the code to:

if(current[prop] === undefined){

Has fixed the problem, but I am unaware of the decision behind using the special util function.

Using: nodejs v0.4.8 now @0.6.1

ericz commented 13 years ago

Hi Nican,

What browser are you using?

Nican commented 13 years ago

Well, the error was being thrown in nodejs, but: Google Chrome 14.0.803.0 (Official Build 90483) dev-m

ericz commented 13 years ago

@Nican, try updating to 0.7.2, the latest version of NowJS on NPM and see if this issue still exists.

ericz commented 13 years ago

Doesn't seem to be a problem anymore. If you're still having an issue with this please let me know @Nican.

Closing