`no-side-effects` prevents caching

[ef4]

Native getters with autotracking don't include any caching by default, and users are encouraged to implement their own caching instead in the cases where caching is important.

But the no-side-effects rule pushes them away from implementing the simplest, clearest form of caching:

get thing() {
  if (!this._cachedThing || this._cachedThing.isStale()) {
    this._cachedThing = this.makeANewOne(); // lint error: Don't introduce side effects
  }
  return this._cachedThing;
}

Are there lots of other ways to build a cache, some of which evade this rule? Yes. But the pattern shown here is safe and any JS dev can easily invent it and is absolutely going to try it anyway, and it's 100% safe and effective in Ember.

[bmish]

Hmm. Allowing caching like this sounds reasonable. But we do still want to disallow other usages of assignment (as that's the entire point of the rule, and I have seen many real-world examples of developers causing all sorts of undesirable side effects).

Perhaps there's a heuristic we can use? If we see something of the form if (!this.foo ...) { this.foo = ... } in a non-computed getter, then we ignore the assignment? Possibly controlled by an option allowCaching.

[ef4]

One idea is to have an option similar to varsIgnorePattern on no-unused-vars:

https://eslint.org/docs/rules/no-unused-vars#varsignorepattern

Then you could customize the rule to allow assignment of properties that match a pattern like /^_cache/.

Getting philosophical for a moment: in Javascript, it's impossible to programmatically tell "good" side effects from "bad" side effects. It's just a fundamentally not-functionally-pure language, so hard and fast rules against side effects can't be enforced, so people must apply judgement to know the difference.

[robinborst95]

I also just ran into the same problem and found this issue. I have a similar property that I want to cache, but mine also allows setting it to a new value. So, by disabling this side effects rule, I wanted to achieved that like so:

@tracked _comment;

get comment() {
  return this._comment ||= this.createComment();
}

set comment(value) {
  this._comment = value;
}

However, this gave the infamous error

You attempted to update `_comment` on `...`, but it had already been used previously in the same computation.

I wanted to make _comment tracked to make sure that setting it to the new value will trigger an update in the template. It probably works with another property like

get comment() {
  return this._comment ?? this.cachedComment;
}

set comment(value) {
  this._comment = value;
}

get cachedComment() {
  return this._cachedComment ||= this.createComment();
}

but I don't really like these extra properties.

However, I do have a simpler solution that works in this case, so I figured I shared it here for anyone who runs into the same thing and finds this issue. As suggested in the Ember guides, we can use the cached decorator for this, and for me that worked like a charm here. So, the final version became:

@tracked _comment;

@cached
get comment() {
  return this._comment ?? this.createComment();
}

set comment(value) {
  this._comment = value;
}