722597 - New Collection module

Status: RESOLVED WONTFIX

(Add-on SDK Graveyard :: General, enhancement, P4)

Description

[Ben Bucksch (:BenB)]

Please include Collections as Jetpack API module

It is described at https://wiki.mozilla.org/Jetpack/Collections
The code is in git at http://git.beonex.com/jetpack/collection/ 

For now maybe as low-level, but the goal is for this API to eventually be declared stable.

I realize that there is an existing low-level module called "collection", but it's not doing much. The new module should provide an API that is compatible, with one exception: I am intentionally not allowing both single items and arrays to be passed to add(). First, because this is a nasty overloading (which complicates implementation and confuses users of the API). Second, it prevents from creating a collection of arrays. I think both are good reasons, but esp. the last is a killer.

I hope that these collections will eventually be used throughout Jetpack, esp. in UI modules, which allows them to seamlessly adapt and update without effort on the part of the programmer.

It is a very powerful, yet lean API, and I am every excited about it. Please do include it.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Slight nitpick: I get '403 Forbidden' for this url: http://git.beonex.com/jetpack/collection/

Comment 2

[Ben Bucksch (:BenB)]

Jeff, you need to
git clone http://git.beonex.com/jetpack/collection/
There is no GitWeb webfrontend, for security reasons.

Comment 3

[Ben Bucksch (:BenB)]

Attachment: git clone https://github.com/benbucksch/jscollection

the attachment here is empty, please fetch
git clone http://git.beonex.com/jetpack/collection/

Comment 4

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Ah, right! I think you would eventually want to put the code into a fork of the addonsdk github repo, then issue a pull request. Probably best to get some feedback first though. :)

Comment 5

[Ben Bucksch (:BenB)]

OK, will do that in due time.

Comment 6

[Ben Bucksch (:BenB)]

I think that should be in core, because it's extremely useful in many widely different cases. (Compare java.util.*.) This builds on top of low-level JS types like Array and Object, but builds a whole set of data types and operations on top of it, which will be widely needed and useful.

Esp. the observers and operators will be very helpful in building UIs, because the programmer doesn't have to update the UI every time that the data changes, but the raw data logic and UI can be separate, but adapt to each other. For that to work, there needs to be common standard on how the logic and UI interact, and standard that's very core to the system and used by all modules. The whole idea doesn't work, if it's not in core.

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Hi Ben,

First of all thanks for significant effort to make SDK better. Please find my feedback below:

1. In general I do like the idea of observable collections type and I do see many benefits described under the wiki. That being said I'm not yet convinced that using them in base SDK APIs will pay off maintenance resource and learning curve introduced. This does not means we don't want it, it means we don't know it yet, we will probably know better once we start rewriting some of the outdated parts of the SDK.

2. Unfortunately we don't have any defined flow how some functionality may end up into core, but we're working on it! While it's not decided yet the way I see it is following:

- Write up a user space library.
- Let it mature.
- Propose it for inclusion into core.

Also keep in mind that we would like to keep core as small as possible and build
ecosystem for user space modules. In other words one should not care much weather
module comes from core or from third party which is trusted.

3. I'd suggest to keep API minimalistic isEmpty => !collection.length, removeAll, addAll are not really necessary.
4. How do you handle `delete collection[index]`.
5. Rather then trying to emulate arrays using  `__iterator__` it would make more sense to me to implement `forEach`, `map`, `filter`, ... and don't provide any other iteration.
6. Observing is a different matter and probably it should be a different thing, maybe `ObservableCollection`.
7. All the observing in jetpack happens via event emitters, so any collection API should be consistent with it. `c.on('add', function() {})` ...
8. `KeyValueCollection` is basically `Object.create(null)` implementing diff API and additional (not sure how useful) `getKeyForValue` method.

9. This proposal does not address following problems that SDK is concerned about:

- Validation of items added to a collection. Something like backbone collection:  http://backbonejs.org/#Collection 
- There are many cases where we expose limited APIs only iteration no add / remove / observe. Or just add no observer / iterate / remove, etc...


I do understand the motivation behind making it part of core, but again we don't yet know what wold fit our use needs most. Neither we do have enough resources to take ownership of this. I'd say best bet for now would be a user space, and we'll try to make user space first class! We may consider pulling this inn into core once we'll have an actual demand on such a functionality.

Comment 8

[Ben Bucksch (:BenB)]

> 1. In general I do like the idea of observable collections type
> and I do see many benefits described under the wiki.

Great!

> This does not means we don't want it, it means we don't know it yet
> we will probably know better once we start rewriting some of the outdated
> parts of the SDK.

OK, thanks for giving me a bit of hope. If I can do something to help, e.g. with the "rewriting some of the outdated parts of the SDK", let me know.

Particularly, I think you will see the value once other modules start using this.
Esp. UI modules. It's nice to see things on the screen automatically, dynamically and immediately adapt to a new situation. I think that's very much in line with the overall JetPack idea, too.

> learning curve introduced.

I tried to make it really easy. The base API is almost identical to the existing |Collection| module, so if you can use that old API, you can use my module. As a normal programmer don't need to use more than add() and remove(), really, if you don't want to learn more.

The learning curve is gradual and hopefully smooth. You can get by with just add(), remove() and contents().

I think the API is also quite logical. The operators are based fairly directly on mathematics and simple concepts like "merge", "subtract" etc..

> That being said I'm not yet convinced that using them in base SDK APIs will pay off
> maintenance resource

FYI, all the code together is less than 1000 lines.

> Also keep in mind that we would like to keep core as small as possible and build
> ecosystem for user space modules. In other words one should not care much weather
> module comes from core or from third party which is trusted.

Yes, I understand that idea, and I like it. It's a good concept for many cases.

Certain things should be in core, though, because they are basic building blocks that all others need to use. E.g. strings, prefs, network I/O, file I/O, and collections.

In fact, a directory listing should return a |Collection|. Right now, the |file| module doesn't give me a way to be notified about new files that are added to a directory. Sooner or later, you would have to add that somehow. With Collections, the programmer doesn't need to learn a new API of how to be notified about new or deleted files, because it's the same API as everywhere else where a list is returned. Of course, of would be optional to use, he can still just do
for each (let file in dir.list())
  debug(file.basename)
but he can also register an observer.

Now, if he can plug dir.list() together with a listbox.showList(coll), it starts to get interesting. The programmer wouldn't need to be registering the observer, but the listbox would do it. (I leave the mapping obj -> appearance to your imagination.)

Unfortunately, this module has no value when standing alone. All its utility comes from being used consistently by other modules. The idea is that completely different modules, like file module and listbox module, have an API that can just be plugged together.

That doesn't work as a third-party module. As a forth-party module writer, I would actively try to avoid depending on such a module from a third-party and rather use JS arrays.

I want to add leanness and power at the same time. I do expect people to like it a lot, if it's in core and they got to use it a bit.

Last but not least: Please note that there already is a Collections module in core. It just doesn't do much and is thus not very useful. I don't know why it was introduced, but I think it was intended to be grown into a class hierarchy like I have provided here.

Comment 9

[Ben Bucksch (:BenB)]

> 3. I'd suggest to keep API minimalistic isEmpty => !collection.length

Fine with me.

> removeAll, addAll are not really necessary.

Agreed. As the doc says, they are just convenience methods. The main reason why I added them was because the existing |Collections| module allows that, too, but overloads add() for that. If an array is passed, it adds all array members. I think that this is a bad idea, not only because it complicates implementation, but more importantly it also prevents from having a collection of arrays.
I can remove these functions entirely, if you wish.

> c.on('add', function() {})

If you think that is the way to go (instead of registerObserver()), I'll follow. Consistency trumpfs.

Comment 10

[Ben Bucksch (:BenB)]

FWIW, as for maintainership, because that was a concern for you, I could take that over long-term, too.

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

What's going on here, Irakli? Is this still something we want to implement?

Comment 12

[Ben Bucksch (:BenB)]

I would still very much like this to be merged.

This should be used as building block for any APIs that have arrays or lists, so that we get change observers for free, which makes e.g. UI code a lot nicer.

Comment 13

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Ben I'm afraid we're not going to merge this in. That being said feel free to distribute this a third party library.

Comment 14

[Ben Bucksch (:BenB)]

Iraki, this module only makes sense as core and as building block for other APIs. The rejection means the work was useless.

Well, it's your power, I can't do anything.

FWIW, that's also why I stopped contributing to Jetpack for other things.

Comment 15

[Ben Bucksch (:BenB)]

Published on https://github.com/benbucksch/jscollection