I wonder if there's anywhere in the docs where we should encourage people to use .fromMap for performance reasons?

As it so happens, I have another PR aimed to improve documentation; I'll definitely be pushing an update there once this guy lands :)

Thanks @nate-thegrate for working on this, expanding the fromMap usage as proposed @justinmc looks like a really good solution for this issue. Also the syntax and usage is much nicer, double win imo, exciting stuff! 😄 👍🏻

Question about this comment:

(Equality checks are still broken for WidgetStateProperty.resolveWith(), and any other non-const objects that implement the interface.)

Did you mean that some resolveWith methods do not yet have a fromMap equivalent and thus we cannot and not use them yet in ThemeData component theming for all resolveWith use cases?

If so, any plans on addressing it?

Or did you mean to that consumers of resolveWith, like the default constructors in component defaults and default themes, will have to replace their own resolveWith themes, with corresponding fromMap versions to get the benefits?

I am bringing this up, because, if custom themes use even a single resolveWithin their theming in any component theme, they break ThemeData equality, as I noted and demoed in the original issue. The hole situation then becomes pointless, as MaterialApp and ThemeData triggered rebuilds won't be reduced if 99% of custom themes use fromMap, but you have just a single resolveWith in there somewhere.

All more involved custom theming in Flutter currently rely heavily on using many different versions of resolveWith() not the most common one WidgetStateProperty.resolveWith((Set<WidgetState> states) {}, things like:

• WidgetStateProperty.resolveWith<Type> with specific types, e.g. type Color and Icon are quite common too.

Then we have specialized ones like e.g.:

• WidgetStateBorderSide.resolveWith
• MaterialStateUnderlineInputBorder.resolveWith
• MaterialStateOutlineInputBorder.resolveWith

They are all used and needed a lot when you make custom themes. Are there now with these updates, fromMap variants for them all that we can use to not break ThemeData equality when we theme?

We really need that. To then teach users to use .fromMap, is of course a matter of documentation and communication.

These would also be a great target for a Flutter lint, to at least detect any resolveWith usage and recommend the move to fromMap. A dartfix might probably be tricky to deliver for it, but would of course be fantastic.

In any cases, awesome work, thanks! 👏🏻 💙

Did you mean that some resolveWith methods do not yet have a fromMap equivalent and thus we cannot and not use them yet in ThemeData component theming for all resolveWith use cases?

Nope! Fortunately, any configuration that can be represented using a resolveWith() callback can easily be converted to a WidgetStateMap. The only caveat is that depending on the logic, sometimes using a callback is the more efficient/less verbose way to do it.

Or did you mean to that consumers of resolveWith, like the default constructors in component defaults and default themes, will have to replace their own resolveWith themes, with corresponding fromMap versions to get the benefits?

Yes indeed! This PR was specifically targeted at the .styleFrom() methods; they should have reliable equality checks from now on.

Then we have specialized ones like e.g.:

• WidgetStateBorderSide.resolveWith
• MaterialStateUnderlineInputBorder.resolveWith
• MaterialStateOutlineInputBorder.resolveWith

They are all used and needed a lot when you make custom themes. Are there now with these updates, fromMap variants for them all that we can use to not break ThemeData equality when we theme?

Yes! Any situation where you could use .resolveWith() also allows .fromMap(). I think there are a couple of constructors I overlooked earlier, but they should be added soon :)

These would also be a great target for a Flutter lint, to at least detect any resolveWith usage and recommend the move to fromMap.

I've considered opening a PR to deprecate the .resolveWith() constructors in order to promote more stable equality checks. But to be honest, I don't think it's a huge deal most of the time.

It's a very noticeable issue when a MaterialApp is rebuilt with different theme data, since its AnimatedTheme triggers a rebuild on every frame throughout the duration, for every widget using Theme.of(context).

But my guess would be that most of the less-experienced developers are going to prefer ButtonStyle.styleFrom() and/or .fromMap() constructors, since (not to brag or anything) they're simpler and easier to work with than the .resolveWith() callbacks we'd been using previously. So even in cases where an equality check could potentially cause a large amount of consecutive rebuilds, it doesn't worry me very much.

Awesome, thanks for the response with clarifications @nate-thegrate 🙏🏻 💙

Looking forward to replacing all the resolveWith with corresponding fromMap in FlexColorScheme and many other projects too, when this lands in stable.

I've considered opening a PR to deprecate the .resolveWith() constructors in order to promote more stable equality checks. But to be honest, I don't think it's a huge deal most of the time.

Might be a good idea to do so. I would be totally onboard with it too, even if it is a lot of re-work for me, but I plan to do it anyway 😄

If you make even a bit more advanced themes, you will have a lot of resolveWith usage. I checked, I have around 78 of them in FlexColorScheme, that is probably extreme though.

Still, getting a nagging deprecation reminder to replace them is even better than a lint. Only way to get rid of the wrongly triggered rebuilds of AnimatedTheme, is to replace all resolveWiths, if you use a single one the issue remains.

This AnimatedTheme and broken ThemeData deep equality is btw an issue I have observed for years when working with theming. So I knew exactly what triggers it and why. That is why I jumped into the issue with explanations of why it happens, even with a simple ThemeData equality test to show the root cause: #89127 (comment)

Seems like some found this second explanation more clear #89127 (comment)

Anyway, thanks again for coming up with a fix for this and as a bonus a much nicer syntax! 💙