Bug 1591973 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Several DevTools actors define `traits`, which act as a flags. 

Most of the time they are created when a new API is added on the actor. This way the client can check if the actor has the new method or not, and fallback properly in case the actor is old (backward compatibility).

Sometimes they are also use for feature detection regardless of backward compatibility concern. For instance the root actor has the `networkMonitor` trait set to true, while the content process target actor has it set to false.

The goal of this meta is to improve the current usage of traits in DevTools codebase. Below are a few suggestions / issue descriptions.

### 1 - Add a consistent API

Generally speaking, traits are returned in the form of an actor, most often in `form.traits`. When the client needs to read traits there is not a single API today:
- target-mixin defines a handy `getTrait` helper which reads traits from the corresponding target actor .... as well as traits from the root actor (which is why contentProcessTargetActor can "override" the `networkMonitor` trait defined on the root actor
- some fronts assign `form.traits` on `this.traits` 
- some fronts just read `this._form.traits` directly (because they save form on this._form)

TLDR; it's quite messy and before you add a new trait, you might have to add the traits to the actor's form as well as doing some plumbing on the front so that you can read your trait. 

It would be nice if this was a built in feature of actors and fronts and if adding traits was easy to do.

### 2 - Provide default traits for supported methods

Say you added a new method on the actor, called "someMethod". Since the client side always sees "up to date" specs, the front can't simply check `super.someMethod`, because thanks to the spec, this method will always be created. That's why 99% of the time we create a trait which has only one purpose: say that the actor implements "someMethod".

For this very simple scenario, it would be nice if by default the traits could list the supported methods of the actor, without having to manually create traits.

### 3 - Automatically detect unused traits

We have a tendency to pile up traits, and we don't clean up old traits after removing all the consumer code. If we had consistent patterns for reading traits on the client `getTrait("hasSomeFeature")` and if we imposed all traits to have a unique name (maybe by declaring them in a shared constant file?), we could add an automated test checking that traits are still used (and if not, suggest to remove them!)

### 4 - Differenciate Backward compatibility and Feature detection

When the consumer code is still available in the client side, we might still want to remove a trait if the JSDoc mentions it was for it was added in an old Firefox version. However we need to be careful that some traits can be used for feature detection as well. I don't have a good solution for this. I think each trait should be accompanied with a good comment to explian why the trait was added in the first place. But a trait added for backward compatibility might later be used for feature detection or vice versa, so I think this is something we will always have to be careful around
Several DevTools actors define `traits`, which act as a flags. 

Most of the time they are created when a new API is added on the actor. This way the client can check if the actor has the new method or not, and fallback properly in case the actor is old (backward compatibility).

Sometimes they are also use for feature detection regardless of backward compatibility concern. For instance the root actor has the `networkMonitor` trait set to true, while the content process target actor has it set to false.

The goal of this meta is to improve the current usage of traits in DevTools codebase. Below are a few suggestions / issue descriptions.

### 1 - Add a consistent API

Generally speaking, traits are returned in the form of an actor, most often in `form.traits`. When the client needs to read traits there is not a single API today:
- target-mixin defines a handy `getTrait` helper which reads traits from the corresponding target actor .... as well as traits from the root actor (which is why contentProcessTargetActor can "override" the `networkMonitor` trait defined on the root actor
- some fronts assign `form.traits` on `this.traits` 
- some fronts just read `this._form.traits` directly (because they save form on this._form)

TLDR; it's quite messy and before you add a new trait, you might have to add the traits to the actor's form as well as doing some plumbing on the front so that you can read your trait. 

It would be nice if this was a built in feature of actors and fronts and if adding traits was easy to do.

### 2 - Provide default traits for supported methods

Say you added a new method on the actor, called "someMethod". Since the client side always sees "up to date" specs, the front can't simply check `super.someMethod`, because thanks to the spec, this method will always be created. That's why 99% of the time we create a trait which has only one purpose: say that the actor implements "someMethod".

For this very simple scenario, it would be nice if by default the traits could list the supported methods of the actor, without having to manually create traits.

### 3 - Automatically detect unused traits

We have a tendency to pile up traits, and we don't clean up old traits after removing all the consumer code. If we had consistent patterns for reading traits on the client `getTrait("hasSomeFeature")` and if we imposed all traits to have a unique name (maybe by declaring them in a shared constant file?), we could add an automated test checking that traits are still used (and if not, suggest to remove them!)

### 4 - Differenciate Backward compatibility and Feature detection

When the consumer code is still available in the client side, we might still want to remove a trait if the JSDoc mentions it was for it was added in an old Firefox version. However we need to be careful that some traits can be used for feature detection as well. I don't have a good solution for this. I think each trait should be accompanied with a good comment to explain why the trait was added in the first place. But a trait added for backward compatibility might later be used for feature detection or vice versa, so I think this is something we will always have to be careful around

Back to Bug 1591973 Comment 0