Remove the annoying const lints from flutter_lints

[goderbauer]

In theory, const-ness should give apps a performance boost, but during development the lints enforcing const are constantly nagging developers: "Make this const" or "This cannot be const anymore". To evaluate whether we are making the right trade-offs here between annoyingness and performance we ran some benchmarks (see flutter/flutter#149932). The benchmarks have not shown sufficient evidence to suggest that there is a statistically significant difference in performance between const and nonconst for real world apps¹. Given these results, the lints that constantly nag developers during development are not carrying their weight and I suggest we remove them from the flutter_lints set. People can still manually enable them for their app, if they like.

Lints suggested for removal from the flutter_lints set (they can still be enabled manually):

• prefer_const_constructors
• prefer_const_literals_to_create_immutables
• prefer_const_declarations

I suggest we keep the forth const lint (prefer_const_constructors_in_immutables) because it is very narrow in scope and generally doesn't cause any pain during development. It also enables downstream consumers to make their own choice of whether they want to create a const instance of a widget or not.

Footnotes

1. In a contrived benchmark that did not represent real world usage in Flutter apps we were able to show that const in general can give you a small performance advantage. ↩

[pq]

/fyi @dart-lang/language-team

[eernstg]

The language team has had discussions about the churn caused by a small modification that changes which maximal enclosing expression can be const. E.g., const [e1, e2, e3] may become [e1, const e2, const e3] if e1 is modified such that it isn't a constant expression any more. The converse is similar: [e1, const e2, const e3] may become const [e1, e2, e3] if e1 becomes a constant expression—except that we may also be able to "move the const keyword out" to an even bigger enclosing expression.

We tried to introduce an "auto-const" feature a long time ago, but we removed the feature again because there were too many difficulties with it. In particular, it's a very bad idea to define a language mechanism in terms of "do a specific thing unless it produces a compile-time error".

Another difficulty is that an expression like <int>[] has a different semantics than const <int>[], which means that a number of expressions may allow for the addition of const, but it introduces a bug in the program. We can't remove const because it disrupts the outcome of identical, and we can't add it because it prevents otherwise available run-time behaviors (like adding new elements to a list).

This means that it is difficult to create automatic and smooth support for "keeping expressions as const as possible". Hence the irritation that motivates this issue.

I think another implication of this is that it is not a very good idea for the maintainers of a package to say "let's turn these lints off for now, never write const anywhere, and then we'll constify everything just before we create a new release of this package". We can't really introduce massive changes in this area on a short notice, no matter whether it's done automatically or manually. It's too error prone, and the potential bugs can be very subtle.

Does this matter when it comes to these lints? Would it be fine if we just end up having a lot fewer occurrences of const? Do we expect the removal of these lints from flutter_lints to cause the constness changes to happen in big chunks rather than continuously, and how much of a problem would that be?

[eernstg]

I created dart-lang/language#4084 in order to push on the possible introduction of a mechanism that would (largely) eliminate the churn that motivates this issue.

[lrhn]

In theory, const-ness should give apps a performance boost

Yeah, I'd be very wary about any blanket statement like that. It's a tool. If used precisely, I'm sure it can improve some performance measures.

It will likely reduce the number of allocations, if equivalent objects would otherwise be created multiple times, but it may also increase the total memory pressure by having a bunch of objects that are never GC'ed. If the objects are pre-created, it could hurt start-up time too. Native AoT has options. Web code compiled to JavaScript needs code to create the object either way, and then needs extra state to remember the canonicalized constants.

Being able to use identical for comparisons instead of a slower property-by-property == implementation can be a benefit. But again, if it doesn't short-cut often enough, then it's an extra step for all the other comparisons.

(A very optimizing compiler could potentially recognize that a class is immutable, its arguments are immutable, and nobody ever uses identical on it, and choose to canonicalize instance of the class created in different places. But I guess if the idea is to use identical instead of ==, then that won't help.)

I'm a little surprised that it has no effect, but I'm not so surprised that the effect isn't big.

[mit-mit]

Please see recent updates in flutter/flutter#149932 (comment)

[caseycrogers]

I give the proposal to remove the const lints a huge +1.

I find using const everywhere where it is applicable to be one of the biggest if not the biggest annoyance in writing day-to-day Flutter. It's especially annoying in the scenario where I have a const children list that I want to add a non-const child to:

return const Column(
  children: [
    Foo(),
    Bar(),
    Baz(baz: getBa...), // <- `getBaz` is not constant so autocomplete refuses to suggest it.
  ],
);

I think efforts to make const lints not annoying/less annoying (eg the aforementioned const? keyword) are admirable, but feel like a very hard circle to square. IMO, the best place to solve dev-ex around const would be improving IDE tooling to make the IDE const/un-const expressions at will as I type so I don't ever have to think about it. But of course still leaving me the ability to opt-out of this behavior at the project, file and expression level for the narrow set of circumstances where const is undesirable.

All the above said, I think the burden of proof should be on the const lints to justify their existence and if they can't demonstrate a strong benefit, they should be opt-in lints, not opt out. We can remove the lints and then if we do find a way to make them not tedious, add them back in. That could be a good bit of thrash, but the odds that the tediousness gets resolved feels very low given the subtle edge cases involved.

[lucavenir]

Isn't it possible to "relax" the analysis constraints, such that an autofix simply removes the const where it isn't meant to be there?

DX wise that would imply that, on save, the consts are auto removed when erroneous, and added when an optimization can be set.

The whole "problem" would disappear at that point, no more going back and forth looking for consts that may or may not be actually consts.

EDIT. I just recognized I basically proposed what @caseycrogers just said 😆 But I disagree on one thing: why should there be a burden of proof on lints? There's nothing to prove: a const expression has its optimizations (no heap reallocation, identical, etc.). Whether or not such optimization suits your use case is situational. But when possible it should be done.

[alefwd]

@goderbauer just out of curiosity, why you'd rather remove the lint rules, than simply disable them? 🤔

[caseycrogers]

...There's nothing to prove: a const expression has its optimizations (no heap reallocation, identical, etc.). Whether or not such optimization suits your use case is situational. But when possible it should be done.

Better in theory or even in synthetic benchmark doesn't necessarily translate to measurably better in practice. The benchmarks at the top of this issue show no measurable perf benefit in real world apps.

Someone should have to prove that const is providing measurable benefit to many projects for the lints to stay opt out. Otherwise, they should be opt in. The more I think about it, I'm not even sure why we're bothering trying to improve dev ex around const if attempts to measure its real world impact show no measurable benefit. If we can't demonstrate benefit in real programs, let's just turn off the lints and move on? It's a bad use of dev resources. Can anyone make a compelling practical argument otherwise?

Fwiw, we only have one production use case benchmarked. If we tested more and found that a lot of real world projects do benefit then that'd change things, but shouldn't we do those benchmarks before even bothering trying to improve const dev ex?

[mit-mit]

@goderbauer just out of curiosity, why you'd rather remove the lint rules, than simply disable them? 🤔

@alefwd: I believe that what Goderbauer means here is to "remove them from our Flutter recommended lints", which is essentially no longer enabling them by default, as those lints are what flutter create uses.

Developers can still enable them if they want to by simply adding the lints to the analysis_options.yaml file.

[gregslu]

'constantly nagging developers: "Make this const"'
In vscode for example, this can be done automatically on save:

"editor.codeActionsOnSave": { "source.fixAll": "explicit", "source.organizeImports": "explicit" },

[lukepighetti]

'constantly nagging developers: "Make this const"' In vscode for example, this can be done automatically on save:

"editor.codeActionsOnSave": { "source.fixAll": "explicit", "source.organizeImports": "explicit" },

So I'm obviously against removing it

why do you want them to begin with?

also, adding const is a tiny fraction of the pain caused by this lint. and a solution that works for your workflow in your IDE is not universal

and furthermore i dont recommend autofix to solve this problem because the linter will recommend something be const, then in subsequent iteration youre stuck with an invalid const that breaks autocomplete in your widget tree. its much better to iterate ignoring the lint and then running a fix all on the codebase before committing

[gregslu]

'constantly nagging developers: "Make this const"' In vscode for example, this can be done automatically on save:
"editor.codeActionsOnSave": { "source.fixAll": "explicit", "source.organizeImports": "explicit" },
So I'm obviously against removing it

why do you want them to begin with?

also, adding const is a tiny fraction of the pain caused by this lint. and a solution that works for your workflow in your IDE is not universal

I like the idea that if something can be const, it will be automatically (in the context of this lint rule, and vscode automatic action on save) because without this rule, you will forget to make stuff const, and if for example main listview item, that will be drawn possibly in hundreds, is not const, and could be, it's huge performance lost. But I agree that mostly it doesn't matter, for performance, for example if simple text widget is const or not, and that for other IDE's it can piss off ppl. For me it works :)

[lucavenir]

then in subsequent iteration youre stuck with an invalid const that breaks autocomplete in your widget tree

@lukepighetti if this could be solved at analysis-time, as I said above, it's a win win for everyone.
The requirements for the analyzer changes would be:

• not to break analysis on an const optimization
• auto fix the removal of non compliant consts
• keep the auto add of consts as is

So nobody loses consts optimizations.
But I'm not sure if it's feasible.

[eernstg]

But I'm not sure if it's feasible.

This one could be of interest: dart-lang/sdk#56669 (comment).

(I'm not quite sure I understand your comment correctly, @lucavenir, but I noted 'auto add of consts' and thought you were considering a mechanism whereby const would be added implicitly during compilation/analysis, and that's what I am focusing on in the comment which is linked above.)

[lucavenir]

This one could be of interest: dart-lang/sdk#56669 (comment).

Yes, absolutely. Thanks.

(I'm not quite sure I understand your comment correctly, ...

Well, actually no. Let me simplify.

Can this diagnostic message include an autofix which automaticall does what the "common fixes" paragraph suggests, i.e. replace with final?

AFAIK there should be a difference: in the comment you've linked you state the analysis cannot tell if something should be const (but it can tell if something can be const, which is fine but unrelated as far as I can tell).

Instead, I'm wondering if an auto fix of the above would suffice. Such auto fix, given that the analysis already knows something cannot be const, removes it and e.g. places a final in its place.

By the way, I love the const? proposal, it's explicit. I like explicitness. The lints / auto fix problems can be easily fixed like that.

[eernstg]

Can this diagnostic message include an autofix which automaticall[y replaces const with final]

That shouldn't be hard to do. Note, however, that it is only applicable to a constant variable declaration. This means that it won't be able to do anything about the main topic of this issue.

Instead, I'm wondering if an auto fix of the above would suffice.

The auto fix could certainly do exactly the same thing in terms of source code transformation as the const? mechanism which is proposed in dart-lang/language#4084 would do as a language mechanism. The difference is that const? would be "re-computed" at every step during development, whereas an auto fix mechanism would need to be applied by a developer (we surely can't allow such a fix to be applied "everywhere it can be applied" because it will change the semantics of the program in subtle ways, so it must be applied manually).

[rayliverified]

I raised this issue on a Flutter Spaces discussion earlier this year so it's great to be vindicated.

Always prefer const should be removed because it causes people to not understand what const does and not use it conscientiously. In addition to the development overhead of the IDE yelling at you.

Did you know that if you want a child widget to rebuild whenever the parent widget rebuilds, all you need to do is NOT mark it as const? Child widgets will automatically rebuild whenever the parent rebuilds, except if the child widget is marked as const. Because of the automatic lint rule, the child widget is automatically marked as const, which leads to many developers adding in additional state management libraries to notify children of parent widget changes. Lots of unnecessary code from a lack of understanding of what const is doing.

As a personal testimonial to the benefit of removing this lint, I've disabled this lint since almost a year and a half ago and it's improved my development speed by at least 10%.

[benthillerkus]

Child widgets will automatically rebuild whenever the parent rebuilds, except if the child widget is marked as const.

Flutter will rebuild a child if the equality check returns false.

const just saves Dart from running the constructor / allocating new objects and it short circuits most equality implementations because they will start by checking identical ie if two references point to literally the same object, which is cheaper than checking each field.

[goderbauer]

To be clear: The lints themselves are not going anywhere. People, that like them, will still be able to opt in to them in their analysis_options.yaml file (just like they can for any other lint). This proposal will only turn these lints off by default for people that use the flutter_lints recommended set of lints (this is the set of lints that's enabled for you if you do flutter create.).

There are probably other things we can do to make the constness less annoying. However, IMO that's only worthwhile if we could proof that const actually gives us a measurable advantage in e.g. performance for real world scenarios. If that's not the case, we should spent those resources on things that give us more bang for the buck.

[natebosch]

Removing from the default set SGTM.

[pq]

I'm in favor of removing these from the default set too.

Assuming there is a real-world performance benefit, I'd love to see us pursue something like @eernstg
explores in dart-lang/language#4084.

[gregslu]

It looks that it would be best to turn off those lints by default, and than run dart fix with them turned on, before committing, but this is not beginner friendly at all xd

[caseycrogers]

It looks that it would be best to turn off those lints by default, and than run dart fix with them turned on, before committing, but this is not beginner friendly at all xd Nubs will not do the second part

This is dicey. There are some circumstances where you may have (intentionally or otherwise) relied on non-constness for an == check and it all worked during dev, but then either manually or as a part of CI/CD you auto-added const and introduced a very subtle bug.

[nate-thegrate]

I'm 100% on board with "encourage const but stop nagging about it during development".

There are some circumstances where you may have (intentionally or otherwise) relied on non-constness for an == check and it all worked during dev, but then either manually or as a part of CI/CD you auto-added const and introduced a very subtle bug.

I ran into this myself recently—I thought giving AppBar a const constructor would be a trivial process, but it turns out that some situations rely on the app bar rebuilding without a dependency update or change to its configuration.

const just saves Dart from running the constructor / allocating new objects and it short circuits most equality implementations because they will start by checking identical ie if two references point to literally the same object, which is cheaper than checking each field.

True, though that only applies to objects with equality implementations.

By default, the object's identity is used to determine equality: if you don't override the == operator, there's no difference between a == b and identical(a, b).

Child widgets will automatically rebuild whenever the parent rebuilds, except if the child widget is marked as const.

This is true, since widgets don't override the == operator.
If they did…

class KeyedSubtree extends StatelessWidget {
  KeyedSubtree({super.key, required this.child});

  final Widget child;

  bool operator ==(Object other) {
    return other is KeyedSubtree
        && other.key == key
        && other.child == child;
  }
}

…evaluating other.child == child would mean that the child (and all of the child's descendants) would have to be evaluated as well. Rather than parsing through the entire subtree, Flutter simply rebuilds any time the new widget isn't identical to the previous one.

There are a few ways to get around this, but the most straightforward is just to use const constructors when possible.

[eernstg]

Sorry about creating some semi-relevant noise on this issue. ;-) Just for the record, I do think it makes sense to remove those const-related lints from flutter_lints.

But it would also be hugely interesting and useful to have more precise information about the actual performance implications of different levels of const usage.

[lrhn]

I have no trong opinion about removing thise from flutter_lints.

I would definitely argue against adding them to recommended, so you can say I'm for removing. I'm just don't write Flutter code, or know why Flutter originally recommended consts, so I don't know what effect it would have to stop encouraging const. (That is: It's really up to the Flutter team if they think it's a net benefit.)

If the reason was just performance, I'd say the difference has to be significant to be worth it. A more targeted lint, maybe only focusing on Widgets, might be more effective.

[alefwd]

@goderbauer just out of curiosity, why you'd rather remove the lint rules, than simply disable them? 🤔

@alefwd: I believe that what Goderbauer means here is to "remove them from our Flutter recommended lints", which is essentially no longer enabling them by default, as those lints are what flutter create uses.

Developers can still enable them if they want to by simply adding the lints to the analysis_options.yaml file.

@mit-mit Michael for sure, thanks! On the package page I read:

This package contains a recommended set of lints for Flutter apps, packages, and plugins to encourage good coding practices

so removing them from there, does it imply all of a sudden const is not a "good coding practice"?
I mean, if he dislike them he could simply disable them. Or at least someone should make an extensive benchmark which claims they are useless.
This is one of the reason I use my own approach, so I'm not influenced by opinionated choices, even when good choices. My approach is to enable all the rules:

https://dart.dev/tools/linter-rules/all

and then disable only the very few which are conflicting. Works like a charm, way better than any package out there IMHO

[mit-mit]

so removing them from there, does it imply all of a sudden const is not a "good coding practice"?

No, I don't think you can negate the statement there and have it mean the opposite.

The goal of the standard lints is to have a starting point for lints which represents a balance between good coding practices and the extra effort it puts on developers. While we want a high coding standard, we also want Dart to be approachable to new developers.

The goal is specifically not to capture all possible app lints. We offer a high degree of customization with lints; so it's expected and perfectly reasonable to pick your own preferred set.

[jezell]

While I don't really have a strong opinion about whether this lint should be in the box, I'm not sure the "real world" benchmark is a great representation of whether const makes a difference. It's mostly just benchmarking if const SizedBox and const EdgeInsets improve perf. As they are trivial widgets, you aren't really gaining much from making them const. There isn't even a single Text widget that is const in the sample because the text widgets are localized:

goderbauer/flutter@4174d22

I believe this points to a larger problem regardless of how the lint issue is decided. It's definitely a fact that reusing widget instances can improve performance, but it's also a fact that due to limitations in dart and flutter, it's hard in practice to use const in places where you'd really desire the widgets to be reused.

If the conclusion is that in the real world you can't ever really make enough widgets const to be useful, then what is the point of having const widgets and flutter pushing people to make all widgets have const constructors in the first place? I believe this "real world" example, while it may not be a good benchmark for if const improves performance definitely shows why in the real world, the limitations that dart and flutter place on const really restrict how much mileage you can get out of it.

[devoncarew]

Thanks all for the feedback on this issue!

For FYI on our standard process for evolving the lint sets:

• proposals are opened as issues, as per https://github.com/dart-lang/lints?tab=readme-ov-file#evolving-the-lint-sets
• periodically, we meet to review. we ask ~3 people to form independent decisions, then meet in a slightly larger group to discuss

The discussion takes into account things like: whether the lint fits into our rubrics for core, recommended, or flutter_lints, whether it's more breaking than we'd want, and whether there are false positives or it would otherwise need more work. We try and have as much of the discussion happen in the open as possible.

For this proposal - because the lints have been enabled for quite a while in flutter_lints - we pinged the framework discord channel for better awareness and asked @jonahwilliams to act as an additional reviewer to leverage his expertise w/ flutter perf issues.

Since @goderbauer, @natebosch, @lrhn, and @jonahwilliams are all in agreement, we'll proceed forward w/ removing the 3 const lints from flutter_lints. Note that this will affect the lint set that new projects start with (and we expect will improve the editing experience for flutter code). As mentioned above in this issue, it does not remove those lints from being generally available; people are still free to add them to their local analysis_options.yaml file and or use other lint sets that include these rules.

[muchirajunior]

So hard to ignore a blue underline on vscode. But personally i don't have any benchmarks if or not the const modifier has app improvements especially that in most cases the app long list of data or state UI will be probably something from the backed and so can't be const. Just the few titles etc. But on another linting issue Why are UI classes forced to be imuttable and marking arguments passed as final yet when you pass a dynamic argument in the constructor it's definately not const.

[quyenvsp]

This will lead to issue flutter/flutter#144511, whole page will rebuild because non-const returned. Now I need add back these removed const rule to analysis_options.yaml. But the problem is not everyone knows this.
Also problem when use String.fromEnvironment