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

Support for read-only observables #220

Closed
eltonomicon opened this issue Jul 10, 2019 · 14 comments
Closed

Support for read-only observables #220

eltonomicon opened this issue Jul 10, 2019 · 14 comments
Labels
enhancement New feature or request

Comments

@eltonomicon
Copy link

I'd like to define stores where observables are only mutated by actions and cannot be mutated externally of the store. This currently is not possible because the code generator defines get/set properties for observables. It would be particularly useful to define read-only observable ObservableList properties since you usually never call the setter once it's initialized.

I know one option is to make private observables then provide read-only computed properties for the observables, but a) that seems unnecessarily verbose, and b) it's not really computed, they're just simple accessors.

For comparison, here's a simple example of how it currently works.

# Current approach
class _TestStore with Store {
    @observable
    int value;
}

# Current generated code
mixin _$TestStore on _TestStore, Store {
 final _$valueAtom = Atom(name: '_TestStore.value');

  @override
  int get value {
    _$valueAtom.context.enforceReadPolicy(_$valueAtom);
    _$valueAtom.reportObserved();
    return super.value;
  }

  @override
  set value(int value) {
    _$valueAtom.context.conditionallyRunInAction(() {
      super.value = value;
      _$valueAtom.reportChanged();
    }, _$valueAtom, name: '${_$valueAtom.name}_set');
  }
}

Here's one suggestion of an alternate approach.

# Alternate approach
class _TestStore with Store {
    int _value;

    @observable(setter: '_setValue')
    int get value
        => _value;

    void _setValue(int value)
        => _value = value;
}

# Alternate generated code
mixin _$TestStore on _TestStore, Store {
 final _$valueAtom = Atom(name: '_TestStore.value');

  @override
  int get value {
    _$valueAtom.context.enforceReadPolicy(_$valueAtom);
    _$valueAtom.reportObserved();
    return super.value;
  }

  @override
  void _setValue(int value) {
    # Same code that was originally in the generated setter
    _$valueAtom.context.conditionallyRunInAction(() {
      super.value = value;
      _$valueAtom.reportChanged();
    }, _$valueAtom, name: '${_$valueAtom.name}_set');
  }
}

I think the ObservableList case as just a read-only property with the default @observable annotation should be sufficient, in that case just generate the getter but not the setter.

@pavanpodila
Copy link
Member

Are you suggesting something like a @readonly annotation where a setter won't be generated:

@readonly
@observable 
int value;

@eltonomicon
Copy link
Author

@pavanpodila I don't think so. If no code at all is generated for a setter then how could value be mutated such that this code would run and trigger a change?

    _$valueAtom.context.conditionallyRunInAction(() {
      super.value = value;
      _$valueAtom.reportChanged();
    }, _$valueAtom, name: '${_$valueAtom.name}_set');

@pavanpodila
Copy link
Member

pavanpodila commented Jul 11, 2019

Ok got it! This will be a custom setter then, whose name you are specifying in the annotation

@eltonomicon
Copy link
Author

Yes, that's what I was suggesting. There may be a better way to declare a readonly observable that can be mutated within the store, that approach was the one I thought was simplest though.

@shyndman
Copy link
Collaborator

shyndman commented Dec 20, 2019

Can't you do this right now by creating an observable on a private field?

class _TestStore with Store {
    @observable(setter: '_setValue')
    int _value = 0;

    int get value => _value;

    @action
    void incrementValue() => _value++;
}

@MisterJimson
Copy link
Contributor

@shyndman that seems to work for me, you don't need the setter param though (it doesn't exist)

class _TestStore with Store {
    @observable
    int _value = 0;

    int get value => _value;

    @action
    void incrementValue() => _value++;
}

@MisterJimson
Copy link
Contributor

It would be cool if @readonly generated the public getter, and you only needed to write

class _TestStore with Store {
    @readonly
    @observable
    int _value = 0;

    @action
    void incrementValue() => _value++;
}

Or even better

class _TestStore with Store {
    @observable(readonly: true)
    int _value = 0;

    @action
    void incrementValue() => _value++;
}

@shyndman shyndman added the enhancement New feature or request label Jan 17, 2020
@mr-mmmmore
Copy link

Adding a @readonly annotation would be great, meaning a public getter and a private setter are generated, so the value is observable and only mutable from inside the store.

@pavanpodila
Copy link
Member

If this feature is really desirable, I suggest we upvote the issue. That way we can prioritize on what we pick up next.

@AlbertoMonteiro
Copy link

@eltonomicon what your thoughts about the ObservableList, I see that this approach is good for simple values, link string or int, but for a complex class, that expose methods that allow changing of itself, how do you think that we could still user ObservableList but only with inside store modifications.

@MisterJimson
Copy link
Contributor

MisterJimson commented Mar 27, 2020

@AlbertoMonteiro you can that complex class be a Store itself. I do this in a bunch of places today.

@AlbertoMonteiro
Copy link

Sorry @MisterJimson but I think that you didn't understand what I said.

@benfgit
Copy link

benfgit commented Oct 11, 2021

@pavanpodila this should be closed.

@Pomis
Copy link

Pomis commented Apr 22, 2022

Why is it limited to private fields? I don't see a scenario, when this could be used, unless you put store and widget in the same file

Could not make class "EmailEditStore" observable. Changes needed:
  1. You should only use @readonly annotation with private properties. Please remove from fields "isSaveButtonActive" and "emailError".

To achieve desired functionality, one has to declare private observable and public computed getter like that:

  @observable
  String? _emailError;

  @computed
  String? get emailError => _emailError;

Would be so much better to have option to do it merely with @readonly annotation

  @readonly
  String? emailError;

---------UPD---------
Didn't realise that public getter is generated for private readonly observables. Leaving comments for those who will have the same misunderstanding

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

No branches or pull requests

8 participants