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

Cannot set now scope variable to null #59

Closed tommedema closed 13 years ago

tommedema commented 13 years ago

When setting a now scope variable on the server side to null, an exception is thrown:

Eg. the following inside some function called by the client on the server (this.now is thus the now scope).

this.now.connectedClient = null;

The following exception is thrown:

{ stack: [Getter/Setter],
    arguments: [ 0, null ],
    type: 'non_object_property_load',
    message: [Getter/Setter] }
  TypeError: Cannot read property '0' of null
      at Object.set (/usr/local/lib/node/.npm/now/0.5.1/package/lib/nowServerLib.js:143:69)
      at native
      at Array.<anonymous> (/usr/local/lib/node/.npm/now/0.5.1/package/lib/wrap.js:41:21)
      at EventEmitter._tickCallback (node.js:126:26)
  DEBUG: process uncaught exception: TypeError: Cannot read property '0' of null
dvv commented 13 years ago

Duplicate of https://github.com/Flotype/now/issues/37

ericz commented 13 years ago

Good catch. We're busier with other issues but will eventually support this

dvv commented 13 years ago

Have you considered switching to CoffeeScript? This would hopefully solve most of code inconsistencies (due to developers own code patterns, such as for-in-ing over arrays, e.g.) and JS quirks -- the resulting code is always linted ok.

erichocean commented 13 years ago

re: coffescript, please don't.

tommedema commented 13 years ago

I'm not a fan of coffeescript either.

@dvv, that issue was closed even though the issue is still here - so I guess this is a "good" replication.

dvv commented 13 years ago

If coffee is not welcome, one should properly iterate over arrays, properly use hasOwnProperty, take care of possible nulls in deep object drill-down, do not forget var and ... and ... Right now there are the said discrepancies in the code, and the staff is working on them, I guess. But the point is to relay onto machine the routine tasks. Coffee to JS is what C to assembler.

tommedema commented 13 years ago

Personally I'm no advocate of variable syncing in general.

I believe real-time communication should be achieved through remote function calls. This would solve a whole lot of issues, make things a lot clearer, and significantly reduce memory vulnerabilties as well as a guarantee that the returned data is up to date.

dvv commented 13 years ago

@tommedema: i guess deleting meaning setting to false is quite ok. delete is operator, not function, so it hardly can be used here. Setting to null/undefined chokes at TypeError: Cannot read property '0' of null which signifies drill-down step is not guarded by a non-null check (of which I said above mentioning CS).

tommedema commented 13 years ago

@dvv, you're right about the operator - well this just shows the complexity and inefficiency variable syncing adds - to be honest it is an unnecessary feature since we've got remote function calls.

dvv commented 13 years ago

on var syncing: @tommedema: So do I. But you couldn't overwise expose functions to the remote end, because functions are always executed at the side which defined them. The point is to be modest -- i found it the best to use a single one-level-deep hash which gets exposed.

ericz commented 13 years ago

You're right guys, variable syncing does introduce a lot of complexity. @dvv we're aware of all the code quality issues, especially inefficient iteration, inadequate null checks etc.

All of the code quality issues and edge cases are issues we're going to tackle before our 1.0 release, which we aim to be at in two months. Right now the priority is experimenting with new features (we are working on many) before we spend time optimizing the code.

One reason for keeping variable sync is that although it is not the perfect process, it allows the users to get started with nowjs very easily. Definitely it is not best practices to rely on variable syncing, it is really only useful for status type values, however it is important to help hide complexity from users.

ericz commented 13 years ago

You can now set variables to null or undefined and it will be properly synced.