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

problem with arrays on everyone #125

Closed tregota closed 13 years ago

tregota commented 13 years ago

I'm having trouble syncing an array to all clients. Setting an array as follows: everyone.now.array = ["test1","test2"]; seems to get synced down to active clients as ["now.array.0","now.array.1"]. any new clients connecting will get the correct contents.

Also using push etc to add to/remove from the same array serverside doesn't seem to do anything.

I'm on Chrome 12.0.742.122 Using version 0.7.3

steveWang commented 13 years ago

Hi,

Am looking into this. It's surprisingly nontrivial.

(Array methods are working after applying a number of hacks, but various other things like JSON.stringify and Array.isArray are more problematic.)

steveWang commented 13 years ago

With commit be4f20604f6c0aae267ecdf78f2b30b7ec7999cd, this is mostly implemented. Just a heads up, JSON.stringify, Array.isArray, and obj.toString don't behave as they should for arrays, but everything else seems to work. Arrays also seem to be reconstructed on the client-side correctly, although that may just be with FF5.

Note that arrays created on the client-side are not yet reconstructed as arrays server-side.

With commit 4701c82786a2dc93f1fc, manually setting an index of an array will set length as necessary. Leaving the issue open in case someone knows how to properly fix JSON.stringify and co.

tregota commented 13 years ago

Great, that works. It's still a bit quirky when initializing with values. setting an array to [1,2,3,4] will get you [now.array.0, now.array.1, etc..] reinitializing an array will overwrite existing values clientside but leave the values with indexes larger than the new array. so [1,2,3,4,5,6,7,8] reset to [a,b] will become [now.array.0, now.array1, 3, 4, 5, 6, 7, 8] on existing clients and for some reason new clients will get the new values and every other of the old ones = [a,b,,4,,6,,8]

steveWang commented 13 years ago

Thanks for the reports! Both issues fixed as of 565d19a42c7638fe4c52.

steveWang commented 13 years ago

Actually, wait, looks like the last part of the second issue hasn't been fixed yet. Looking into it.

steveWang commented 13 years ago

Fixed with 70e80d8433c4c2ce4d11.

evanw commented 13 years ago

I just hit this today. This appears to be fixed but npm still has the buggy version. Could the package in npm also be updated?

tregota commented 13 years ago

Now we are getting somewhere. I happened to notice that the same problem still exists in nested arrays/hashes though.

steveWang commented 13 years ago

evanw : I'd rather have this be pushed to npm once I've resolved as much as possible. tregota: looking into it, will see if I can reproduce and fix. :) edit: I can't find this bug with nested arrays/hashes. I'll add some more debugging, but in the meanwhile, what exactly are you experiencing?

tregota commented 13 years ago

Ahh yeah look at that, it works. I must have messed up the pull or the restart of the server process. Sorry about that. Great work by the way. Keep it up.

Edit: One thing though. If I splice away an element on the server, that will not resize the array on the client side. it'll end up with a null on the end instead.

steveWang commented 13 years ago

The vast majority of these bugs have been fixed as of the latest github commit -- Array.isArray and Object.prototype.toString.call still don't work, but there really is no way to fix these.

New integration tests indicate client-to-server and server-to-client synchronization both work. As such, I'll be marking this issue as resolved.