Bug 709382 require("simple-prefs").prefs should be iterable

[erikvold]

https://bugzilla.mozilla.org/show_bug.cgi?id=709382

[mykmelez]

preferences-service is based on Preferences and is intended to be a cleaner, more friendly, and more JavaScript-y API than the underlying XPCOM interface. I'd like to maintain that friendliness as we add new functionality to it.

In particular, instead of introducing a "class method" that takes a branch name, we should make the exports object be a constructor that retains its existing class methods but also takes a branch name when called as a constructor (just like in Preferences) and from which consumers can retrieve the keys for the branch. And rather than calling the key retrieval method by a non-standard name like getChildList() that comes from the underlying XPCOM API, we should use the standard Object.keys() (like in simple-prefs).

So the API would then look like:

let Prefs = require("preferences-service");
let myPrefs = Prefs("foo.bar.baz");
console.log("My prefs are: " + Object.keys(myPrefs));
// It's still possible to call Prefs.get("foo.bar.baz.qux"), etc.

for.. in can be made to work on these instances, as can branch-scoped versions of the existing class methods like get() and set() (just like in Preferences), but those can happen in a separate change; they don't need to be part of this one.

[erikvold]

Mind taking a look at this one @Gozala ?

[gregglind]

I cleaned this up some and rebased it at #442

(I don't feel like I have authority to close requests here :) )

[Gozala]

Could you please change this to Branch(name) instead ?

[erikvold]

got it dc0140b

[Gozala]

Could you please rename pref, val to key value.

[erikvold]

got it.

[Gozala]

Could we please tackle iterability separately from observability ?

[Gozala]

Do you plan to address this as well #300 (comment) ?
Also, pull request is out of date you need to merge with current master.

[Gozala]

Thanks for doing all of these BTW!

[erikvold]

merged with master now.

[erikvold]

I think this is ready for another review @Gozala

[Gozala]

Please do not alias set / get / keys, ... on Branch constructor.

[Gozala]

Also we should stick to following export pattern:

function foo() {
  // ...
}
exports.foo = foo;

[Gozala]

Also please make sure to Proxy handler code to use method(branchName + pref) instead of Branch.method(branchName + pref) or exports.method(...

[erikvold]

got it 07540b3.

[Gozala]

Just a small details left. Thanks ;)

[erikvold]

got the rest in 082622f @Gozala

[Gozala]

Looks good, thanks a lot!