Need way to subscribe to built instance changes

[moodysalem]

Issue

When writing angular components, typically we want to take as @Input an immutable built value instance, and as @Output we want to push new immutable built value instances with any modifications. This is ideal for developer UX because the angular component does not share references to any mutable data structures.

Doing this with built value is awkward because we have no way to subscribe to changes to a built value builder instance. For a form component, for every field modified in the form, we have to write a method in the angular component that updates the input and adds the new model to the output stream. If there are 10 inputs in the form, that's 10 methods that are almost the same. You can share the common pieces of these methods, but it's still repetitive and adds a lot of methods to the component.

Example

@Component(
  selector: 'some-form',
  template: '''
  <input [ngModel]="thing.a" (ngModelChange)="changeA(\$event)">
  <input [ngModel]="thing.b" (ngModelChange)="changeB(\$event)">
'''
)
class SomeForm {
  @Input()
  BuiltThing thing;

  @Output()
  Stream<BuiltThing> thingChange;

  void changeA(Object value) => thingChange.add(thing.rebuild((b) => b..a = value)));
  void changeB(Object value) => thingChange.add(thing.rebuild((b) => b..b = value)));
}

Suggested fix 1

Optionally generate an additional class that is immutable, called ThingRebuilder, that exposes a Stream<ThingRebuilder> onChange and a setter for each field which outputs modified ThingRebuilders to the stream. This class can have its properties 2 way bound to the inputs.

Suggested fix 2

Add a Stream<void> change listener to the generated builder. That stream is triggered after any setter is called on the builder. This makes it so we can use a builder instance internally in the component and output to the stream the result of thingBuilder.build() whenever the builder is modified.

[davidmorgan]

Thanks.

You could add code to the constructor of BuiltThing that runs whenever a new one is created:

StreamController<BuiltThing> thingsController = StreamController();
Stream<BuiltThing> get things => thingsController.stream;
class BuiltThing implements Built<BuiltThing, BuiltThingBuilder> {
  BuiltThing() {
    thingsController.add(this);
  }
}

...but that is then a global variable. Maybe if the model is only used once in your app, that's enough?

[moodysalem]

What about for editable lists of models? That's a pretty common case where you'd have multiple instances of the form.

[davidmorgan]

You could give the model a field which is a StreamController<Thing>. You'll want to mark it @BuiltValueField(compare: false) to stop it being used in hashcode/equality calculations.

Even simpler, you could give it a field of type void Function(Thing), and just pass this to the function in the constructor.

abstract class BuiltThing implements Built<BuiltThing, BuiltThingBuilder> {
  @BuiltValueField(compare: false)
  void Function(Thing) get updateNotifier;
  BuiltThing() {
    updateNotifier(this);
  }
}

Maybe that will do what you need?

[moodysalem]

I think I prefer how the given example looks to adding a field to my built value that is only used in forms-it seems hacky. A change stream on the generated builder seems like it would be intuitive-maybe it could be generated based on a parameter in the annotation?

[davidmorgan]

The builder would still need to know where to get the stream controller from, though--it's the same problem. I can't think of anywhere to get it from apart from a field in the model.

[moodysalem]

I don't understand-wouldn't the generated code have a StreamController in every builder instance? i.e. not exposed to the user of the builder. And then you could access the stream builderInstance.onChange to get notified whenever a setter is called on the builder.

The example would then look like this (suggested fix 2)

@Component(
  selector: 'some-form',
  template: '''
  <input [(ngModel)]="builder.a">
  <input [(ngModel)]="builder.b">
'''
)
class SomeForm {
  ThingBuilder builder;
  StreamController<BuiltThing> _changeController = StreamController();

  @Input()
  void set thing(BuiltThing thing) {
    builder = thing.toBuilder();
    builder.onChange.listen(_handleChange);
  }

  void _handleChange(_) => _changeController.add(builder.build());

  @Output()
  Stream<BuiltThing> get thingChange => _changeController.stream;
}

Not sure if this would be any good for performance, and I think you'd have to cancel the subscription with every new thing input passed in as well

[davidmorgan]

Thanks, I think I'm starting to understand what you need.

For performance, well, it'll force a build on every change, which doesn't normally happen; that isn't hugely expensive, but it does mean creating an object and copying the fields into it.

The two streams in your example seems like overkill; we could instead give the builder a method to call on change

builder = thing.toBuilder();
builder.onSet = _changeController.add;

--this seems reasonably straightforward, but it's a bit annoying that any API we write might clash with field names. I suppose that's okay if we also guard it with an annotation

@BuiltValue(generateOnSet: true)
abstract class Thing extends Built<Thing, ThingBulider> {
...
}

and then this would create the mutable field onSet, initially null; if you set it to contain a void Function(Thing) then the function will be called whenever a set happens.

How does that sound?

[moodysalem]

In the example

builder.onSet = _changeController.add;

This implies onSet has the signature void OnSet(BuiltThing thing). Is that what you're thinking? Not sure what the best signature for that field is, but I wouldn't necessarily want to trigger build() for every set.

This solution works for the described use case. It's also better than the streams because you only are allowed one subscriber by definition. I like the name onChange better than onSet, but either one works.

[davidmorgan]

Hmm, can you think of any alternatives to triggering build() for every set?

[moodysalem]

I would just make it pass zero arguments-I just mean triggering build() by default might not be right for the builder but it would still be necessary in this use case

[davidmorgan]

That makes sense.

I'll see if I can add this in the next round of improvements--should be easy enough.

Thanks!

[deadsoul44]

If there is nested built classes or collections, is onset function called when there is only a change in the deepest built class or collection?

[davidmorgan]

A nested builder doesn't do anything to alert the outer builder about a change, so, only the inner builder onSet will be called.

[deadsoul44]

So I can set a function for every nested builder to add the outer most thing to the stream but onSet is not available for built_collection as far as I understand from this issue: google/built_collection.dart#184 (comment)
and there is no way to alert for a change in a built_collection?

[davidmorgan]

Good point, built_collection is a nested builder by default so it won't trigger onSet.

[deadsoul44]

I am following the issue in built_collection repo. I hope it will be implemented very soon. It will be very useful to be able to use built_value with MobX. Thank you.