eslint-plugin-ember
eslint-plugin-ember copied to clipboard
[BUG] require-computed-macros can autofix to self-referential macros
Given the following (admittedly incorrect) computed usage
storeIds: computed("storeIds", "line", function() {
return this.storeIds;
}),
This rule will convert it to the following, which blows up since the self-referential nature becomes more evident when an alias is installed under the hood.
storeIds: computed.reads('storeIds')
A better conversion would be
storeIds: null
I wonder if it's worth creating a separate rule no-self-referencing-computed-properties
?
@bmish for the first example, absolutely. But since an existing rule also modifies that first example we'd have to make sure they didn't interfere with each other. It's also unlikely you end up with a self-referential computed using a macro without using autofix since it results in application errors almost immediately. Running this on a large app turned out to be problematic since now I'm chasing down self-referentials. Trying to decide whether to patch this rule to convert them better or if I submit a new rule.
I actually think the new rule no-self-referencing-computed-properties
should catch both macro and non-macro computed properties. No reason to limit it to just one computed property form. This new rule would serve to make it clear that something is likely seriously wrong with the self-referencing computed property in a way that requires user attention.
If we make any changes to require-computed-macros
, I would just remove the autofix for this situation. I would not consider it acceptable to autofix to storeIds: null
as that's a behavior change and hides the fact that something is broken and needs manual attention.