Some thoughts on EquatableConfig

[lbel]

Is your feature request related to a problem? Please describe. I really like being able to configure my Equatable classes globally using EquatableConfig (#68), since it saves me a lot of stringify overrides in the various classes. However, I'm worried about the global, shared state of this class.

First of all, there's no logical place for me to put the statement EquatableConfig.stringify = true; – every single test has its own main class, and in addition my project has various main classes for different environments, which leads to a lot of code duplication and makes it very easy to forget this call somewhere. I can also easily mess up by adding it in the unit tests but then forgetting it in the application, which basically invalidates all the toString calls in my unit tests.

Secondly, and more worryingly, any part of my code can now change this static value, which suddenly causes a behaviour change in all of my Equatable classes. This is dangerous from reproducibility perspective (for which we can't even test, because the value might be different in the unit tests), as well as from a security perspective: any part of my code can now make change other classes to have sensitive data show up in my logs, or hide the data I want from my logs, by simply changing this one variable. Currently, the only safeguard against this behaviour is overriding stringify in every Equatable class, which defeats the whole purpose of having this configurable field in the first place.

Describe the solution you'd like The first part of this problem can be solved in a non-breaking way, by defaulting EquatableConfig.stringify to a compile-time constant, eg. EquatableConfig.stringify = bool.fromEnvironment('equatable.defaultStringify', false);. This would already save a lot of trouble in getting the change consistent across my various main classes without having to specify them. However, this doesn't solve the reproducibility/security issue, unless the value were to be made const – but that'd be a breaking change and possibly make a lot of people very sad. It also doesn't guarantee that the production behaviour is identical to what was run in the unit tests, but at least it makes it a lot harder to randomly mess it up somewhere in the code.

I'm not entirely sure if I'm making a valid point or if I'm just spewing nonsense here, so I'm very curious to hear the developers' opinions about this! And as a final remark, I love the project and use it extensively, so thanks for all the work that went into it :) really appreciate it!

[felangel]

Hi @lbel 👋 Thanks for opening an issue and for the positive feedback! I really appreciate it 🙏

I think the compile-time constant enhancement makes sense and I would love to hear what others think about making the value const (cc @jorgecoca).

[vinceramcesoliveros]

I'm thinking of a separate issue regarding EquatableConfig. In your example code

import 'package:equatable/equatable.dart';

class Person extends Equatable {
  final String name;

  const Person(this.name);

  @override
  List<Object> get props => [name];

  @override
  bool get stringify => true;
}

this prints Person(Bob). But when it has too many (named)parameters it can be hard to determine which values have been set to null or numbers we referred to(like latitude & longitude). I really hope we can get the variable names like this -> Person(firstName: Bob, secondName: Martin).

[zepfietje]

@felangel, I agree with @lbel that having to specify EquatableConfig.stringify = true; in every test still causes quite a bit of code duplication. Basically, we've moved the stringify code from the implementing Equatable classes to our tests...

[felangel]

@ram231 I don't think that's possible without code-generation unfortunately.

@zepfietje Can you please explain why stringify needs to be enabled for tests? Do you have explicit tests to ensure toString returns the correct String?

[zepfietje]

@felangel, stringify actually takes away the need to test for a correct toString. However, when testing blocs, stringify is really useful for debugging failing tests. Otherwise, you can't really distinguish the actual and expected emitted states for example.

[felangel]

@zepfietje got it, thanks for clarifying 👍

[normancarcamo]

Hi there' I was about to ask this question, luckily for me I've found this, I'm thinking of using this config directly into the main.dart file. I think it's a good place, what do you think?