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

The length of an array should not be observed #5

Closed
louisameline opened this issue Mar 13, 2015 · 12 comments
Closed

The length of an array should not be observed #5

louisameline opened this issue Mar 13, 2015 · 12 comments

Comments

@louisameline
Copy link

The native O.o does not return a change event for the length property of arrays.

@louisameline louisameline changed the title The modification of the length of an array should not be observed The length of an array should not be observed Mar 13, 2015
@louisameline
Copy link
Author

Oops, I should have said :

"It would be great if your library polyfilled Array.observe as well, it's the same except that non-enumerable properties should not be included" :)

Thank you.

@MaxArt2501
Copy link
Owner

Yes, it does. I tried in both Chrome and io.js. Use this snippet:

var a = [];
Object.observe(a, console.log.bind(console));
a.push("foo");
// [ { type: 'add', object: [ 'foo' ], name: '0' },
//   { type: 'update', object: [ 'foo' ], name: 'length', oldValue: 0 } ]

Unless, of course, I misunderstood what you mean.

@MaxArt2501
Copy link
Owner

Oh, ok, I posted second after your update :)

Array.observe has been taken under consideration, but there are some issues that need to be tackled first, and I still have to decide on the matter. Namely, the splice change cannot be completely polyfilled.

For the cases it can effectively be polyfilled, there's the caveat of changing the Array prototype. I wrote a snippet in the documentation of how it can be done.

@louisameline
Copy link
Author

Thank you for your super-quick answer !

Sorry, I missed that part of the doc :s Very interesting, you know your subject inside out :)

I see that you seek perfection and that's admirable, but I personnaly wouldn't be worried about the splice events and wouldn't try to change the Array prototype at all for push and all. It's true that we lose some semantics about how an array was added/removed/updated, but I guess most people are more interested by the mere fact that it's now in the array.

The guys at observe-js, whom I believe also made the spec for some of them, even report update events as delete+add events to the user. I think it's a shame to ignore semantics like that and, although it can be quite handy for most people (I added a noArrayUpdate option myself in my lib). Just to say, we just might be the only ones to care because I haven't seen anybody complain about it in their GitHub issues.

@MaxArt2501
Copy link
Owner

A way to avoid messing with native prototypes would involve changes in the property checker itself. It should take special measures for objects that have the length property (but not all - not HTMLCollection lists for example - ugh) and an algorithm that can tell when an add/update/delete change is actually a splice. Things may get very complicated here.

@louisameline
Copy link
Author

You know more about these things than I do, but I see no algorithm that can truly distinguish a splice when updates are involved.

var arr = ['foo'];
arr[0] = 'bar';

// equivalent but semantically not equal to

var arr = ['foo']
arr.splice(0,1, 'bar');

As you probably know, observe-js uses a distance algorithm to detect splices as much as possible: https://github.com/Polymer/observe-js/blob/master/src/observe.js#L1303

@quantuminformation
Copy link

Chrome observes length on arrays.

@astoilkov
Copy link

This polyfill is amazing. Great work! I was exploring other options and nothing comes close to this. The solution is the best out there.

I want to give a suggestion for the Array.observe functionality. Why don't you do Array.observe.js as separate polyfill(file) and note the problems with it. I am currently working on a project that will use Array.observe and it will be great to have a polyfill to get it to work for other browsers.

If you know a great polyfill for Array.observe I would love to hear about it.

@MaxArt2501
Copy link
Owner

No, I'm not aware of any close polyfill for Array.observe. The usual solutions involve some kind of arrangement with a different API, like Polymer's ArrayObserver - which is always worth considering.

I've been thinking about a separate polyfill for Array.observe, actually. But then I'd have to deal with the fact that Array.observe(...) is just like Object.observe(..., [ "add", "update", "delete", "splice" ]), and Array.unobserve behaves exactly like Object.unobserve (although O.u !== A.u).

Now, should those two polyfill be independent or not? On the basis of what I just said, they can't, since they should operate on each other's observed objects. This would mean finding a (rather awkward) way to making them interoperate, like publically exposing the Maps of observed objects and listeners.

If the polyfills are conceived as independent, they'd lose the method equivalence stated above, forcing the developers to strictly using Array.observe/unobserve when observing arrays - which could be fine in most cases, as long as they actually have a working shim - but also replicate all the internal dirty checking engine for the add, update and delete events. And you may guess I'm not very fond of it either.

As soon as I have a weekend that doesn't gobble all my spare time in other stuff, I think I'll spend some of it brainstorming on the problem and (finally) putting down some code.

In the meanwhile, all the suggestions are welcome!
And thank you for the nice words.

Edit: also I'm quite eager to try jsblocks sooner or later - looks veeeery nice! Kudos on the excellent work.

@MaxArt2501
Copy link
Owner

Opened issue #10 for Array.observe.

@MaxArt2501
Copy link
Owner

@astoilkov I followed your advice for the time being, and created a separate polyfill for Array.observe.

I actually hope it will be short-lived, as that would mean I'll come with a decent solution soon(ish).

@astoilkov
Copy link

Wow. Great work. I took a look at the code and it looks really simple. I am happy to see this thing implemented. You are the first to do something like this. Can't wait to try it out in one of my projects.

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

No branches or pull requests

4 participants