proposal: `no_store_flutter_inherited_property`

[dnfield]

no_store_flutter_inherited_property

(I am not super happy with this name and open to suggestions).

Replace above with your proposed rule name. (See notes on naming in Writing Lints.)

Description

Prevent State classes in Flutter from storing the child properties of inherited objects. I made this mistake and fixed it in https://github.com/flutter/flutter/pull/133503

Details

Storing the child property of an inherited object will not update that property value when the object itself updates. When subscribing to an inherited object, child properties must be accessed freshly every time the object fires an update notification.

Kind

Guard against errors.

Bad Examples

class MyWidgetState extends State<MyWidget> {
  late Size _size;

  @override
  void didChangeDependencies() {
    super.didChangeDependencies();
    // _size will not get updated when the media query size actually changes.
    _size = MediaQuery.of(context).size;
  }
}

Good Examples

class MyWidgetState extends State<MyWidget> {
  late MediaQueryData _data;

  @override
  void didChangeDependencies() {
    super.didChangeDependencies();
    // Data will get updated every time media query changes, and _data.size
    // will always contain the correct value.
    _data = MediaQuery.of(context);
  }
}

class MyWidgetState extends State<MyWidget> {
  late Size _size;

  @override
  void didChangeDependencies() {
    super.didChangeDependencies();
    // Special accessor on MediaQuery that fires only when size changes.
    _size = MediaQuery.sizeOf(context);
  }
}

Discussion checklist

• [x] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• [x] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• [x] If there's any prior art (e.g., in other linters), please add references here.
• [ ] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
• [x] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

[srawlins]

Can you explain more what counts as "storing"? Does assigning to a local variable count? Does passing to an instance setter count? What about passing to an instance setter of an object other than this? Static setter? Passing to a top-level library setter? Does passing to a function as an argument count (e.g. someList.add(MediaQuery.of(context).size)?

I don't understand what is bad about the BAD example. You write, "Storing the child property of an inherited object will not update that property value when the object itself updates." and then in the BAD example, you have _size = MediaQuery.of(context).size;, and I don't see any inherited object there. MediaQuery.of(context).size is (presumably) a getter access on an instance creation expression?

[dnfield]

The example I gave might not be ideal, because the size property on MediaQueryData is final. But imagine that it wasn't, and that MediaQuery might update it without causing dependencies to change - you could end up with a stale size. This can, in fact, happen with FlutterView.display, which is what originally caused me to file this issue.

In the good example, we don't store a child property of the MediaQueryData, in the bad example we do. Passing to a function argument shouldn't count, but passing to an instance setter, static setter, or top-level library setter is probably an anti-patern and should require some thought about whether it's a good idea and an explicit // ignore if it really is a good idea.

[srawlins]

Thanks @dnfield that super clears it up.

MediaQueryData is not a State or a Widget or anything; its supertype is Object. Is the proposal then to never store (into a field etc) the value obtained from an instance getter on any kind of class?