Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot rebuild a BuiltSet entry in BuiltMap #249

Closed
lukepighetti opened this issue Jul 27, 2021 · 7 comments
Closed

Cannot rebuild a BuiltSet entry in BuiltMap #249

lukepighetti opened this issue Jul 27, 2021 · 7 comments
Labels

Comments

@lukepighetti
Copy link

lukepighetti commented Jul 27, 2021

It appears that if you try to rebuild a BuiltSet which is an entry of BuiltMap, it does not rebuild as expected. The changes are not applied.

main.dart

import 'package:built_collection/built_collection.dart';
import 'package:built_value_map_entry_rebuilder/models.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
  test('BuiltMap entry update', () {
    final joe = Person((b) => b..name = 'Joe');
    final luke = Person((b) => b..name = 'Luke');

    final org = Organization((b) => b
      ..name = 'Foolandia'
      ..people = MapBuilder({
        'mobile': BuiltSet<Person>({joe, luke}),
      }));

    final orgBuilder = org.toBuilder();

    /// Initial works
    expect(orgBuilder.people['mobile'], equals(BuiltSet<Person>({joe, luke})));

    /// Weird rebuild works
    orgBuilder.people['mobile'] = (orgBuilder.people['mobile']!.toBuilder()
          ..removeWhere((e) => e.name == 'Joe'))
        .build();
    expect(orgBuilder.people['mobile'], equals(BuiltSet<Person>({luke})));

    /// Rebuild fails, expected to work
    orgBuilder.people['mobile']!
        .rebuild((b) => b..removeWhere((e) => e.name == 'Luke'));

    expect(orgBuilder.people['mobile'], equals(BuiltSet<Person>({})));
  });
}

models.dart

import 'package:built_value/built_value.dart';
import 'package:built_value/serializer.dart';
import 'package:built_collection/built_collection.dart';

part 'models.g.dart';

abstract class Person implements Built<Person, PersonBuilder> {
  static Serializer<Person> get serializer => _$personSerializer;

  Person._();
  factory Person([void Function(PersonBuilder) updates]) = _$Person;

  String get name;
}

abstract class Organization
    implements Built<Organization, OrganizationBuilder> {
  static Serializer<Organization> get serializer => _$organizationSerializer;

  Organization._();
  factory Organization([void Function(OrganizationBuilder) updates]) =
      _$Organization;

  String get name;

  BuiltMap<String, BuiltSet<Person>> get people;
}

built_value_test.dart

import 'package:built_collection/built_collection.dart';
import 'package:built_value_map_entry_rebuilder/models.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
  test('BuiltMap entry update', () {
    final joe = Person((b) => b..name = 'Joe');
    final luke = Person((b) => b..name = 'Luke');

    final org = Organization((b) => b
      ..name = 'Foolandia'
      ..people = MapBuilder({
        'mobile': BuiltSet<Person>({joe, luke}),
      }));

    final orgBuilder = org.toBuilder();

    /// Initial works
    expect(orgBuilder.people['mobile'], equals(BuiltSet<Person>({joe, luke})));

    /// Weird rebuild works
    orgBuilder.people['mobile'] = (orgBuilder.people['mobile']!.toBuilder()
          ..removeWhere((e) => e.name == 'Joe'))
        .build();
    expect(orgBuilder.people['mobile'], equals(BuiltSet<Person>({luke})));

    /// Rebuild fails, expected to work
    orgBuilder.people['mobile']!
        .rebuild((b) => b..removeWhere((e) => e.name == 'Luke'));

    expect(orgBuilder.people['mobile'], equals(BuiltSet<Person>({})));
  });
}

Log

Expected: _BuiltSet<Person>:[]
  Actual: _BuiltSet<Person>:[
            _$Person:Person {  
            name=Luke,  
          }
          ]
   Which: at location [0] is _BuiltSet<Person>:[
            _$Person:Person {  
            name=Luke,  
          }
          ] which longer than expected

package:test_api                                   expect
expect
package:flutter_test/src/widget_tester.dart:441
main.<fn>
test/built_value_test.dart:38
2

✖ BuiltMap entry update
Exited (1)
@dave26199
Copy link

dave26199 commented Jul 28, 2021 via email

@lukepighetti
Copy link
Author

Does that mean that BuiltSet.rebuild() is always no-op? If so, should it throw an error or a warning similar to when you try to mutate an UnmodifiableListView?

@davidmorgan
Copy link
Contributor

It returns a new result, it doesn't modify the existing collection.

I think there's an annotation in package:meta we could use to catch cases where you ignore the result, but I didn't look at that yet--it seems a good idea.

@lukepighetti
Copy link
Author

lukepighetti commented Jul 28, 2021

It seems like there is an API divergence between built values and built collections. If you rebuild a value it mutates the existing instance, while if you rebuild a collection it does NOT mutate the existing instance. Am I understanding that right?

@davidmorgan
Copy link
Contributor

They're the same, built_value classes are also immutable :) ... only builders are mutable.

@lukepighetti
Copy link
Author

Brain fart on my part. Thanks for sticking with me!

@davidmorgan
Copy link
Contributor

No worries, this question comes up regularly :) ... I should try that annotation ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants