Properly distinguish objects from classes

[mjethani]

Background

There are a number of objects in core that share some of their state and provide some methods for querying and modifying the state.

The FilterStorage object in lib/filterStorage.js, for example:

exports.FilterStorage = {
  initialized: false,

  loadFromDisk()
  {
    // Load subscriptions from disk.
    this.initialized = true;
  }
};

These objects are incorrectly referred to as "classes" in the documentation, even though they are just singleton objects. Furthermore, their names start with upper case letters (e.g. FilterStorage as opposed to filterStorage), which makes it even more confusing to reason about these objects. They also use the this keyword internally.

There are two solutions:

1. Make them proper singletons so that they are truly the sole instances of their respective classes. For example, the filterStorage object could be an instance of the FilterStorage class.

2. Let them remain standalone objects (or "modules", if you will), not related to any class, and stop using the this keyword internally to avoid confusion.

Note that in the latter case JSDoc does not appear to correctly associate the object's properties with the object; there is no documentation generated for the properties.

What to change

After #6854, rename FilterNotifier in lib/filterNotifier.js to filterNotifier.

Rename FilterStorage in lib/filterStorage.js to filterStorage. The current implementation of FilterStorage should be wrapped into an ES2015 class (#6741), and the filterStorage object should be an instance of this class.

Integration notes

The FilterNotifier object exported by lib/filterNotifier.js is now renamed to filterNotifier. It is now directly an instance of EventEmitter, with no intermediate object in its prototype chain (i.e. filterNotifier.construtor is EventEmitter and filterNotifier.__proto__ is EventEmitter.prototype, though these internal details shouldn't matter).

After ​changeset 58b0ccc6c289, the FilterStorage object exported out of lib/filterStorage.js is now called filterStorage instead.

Hints for testers

For the dependency update in #6933, there is no change in behavior here, therefore there is nothing specific to test (the current test cases should cover it if there has been any omission in the dependency update). Watch out for errors in the background page console.

[abpbot]

A commit referencing this issue has landed:
​Issue 6891 - Rename FilterNotifier to filterNotifier

[greiner]

I've created #6907 for the corresponding adblockplusui changes.

Are we expecting any more changes of this kind to be added to the next major release? If so, it'd be great if we could reduce the overhead associated with each small change by grouping the necessary UI changes together.

[mjethani]

Yes, I think there will be more changes for #6891 and #6741 (both works in progress), as well as possibly #6559, for the next major release; for #6891 there will definitely be some renaming of things to do.

[abpbot]

A commit referencing this issue has landed:
​Issue 6891, 6741 - Rename FilterStorage to filterStorage

[greiner]

Fixed typo in description.

[Ross]

Should we verify this ticket yet? Or is it still open? I haven't seen any errors in the background console so far.

[mjethani]

Replying to Ross:

Should we verify this ticket yet? Or is it still open?

This is going to remain open until all the changes are done in core. You could verify the dependency update in the browser extension instead (e.g. #6933).

[mjethani]

Replying to mjethani:

Replying to Ross:

Should we verify this ticket yet? Or is it still open?

This is going to remain open until all the changes are done in core. You could verify the dependency update in the browser extension instead (e.g. #6933).

So just to be clear, I don't think there's anything specific to test related to this ticket for #6933.

[abpbot]

A commit referencing this issue has landed:
​Issue 6891, 6741 - Rename FilterStorage to filterStorage

[mjethani]

I am closing this now since it has already been marked verified. I'll open a new one for further changes.