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

Feat: Allow a custom equals parameter for ObservableStream #771

Merged
merged 4 commits into from
Oct 12, 2022

Conversation

amondnet
Copy link
Collaborator

@amondnet amondnet commented Mar 8, 2022

Create a custom equals parameter for ObservableStream, like what exists already for Observable.

Currently, ObservableStream uses == to determine if the value changed. This creates an option to override that with a different EqualityComparator.

if (newValueDynamic == WillChangeNotification.unchanged) {

equals == null ? prepared == value : equals!(prepared, _value);

#679

@netlify
Copy link

netlify bot commented Mar 8, 2022

Deploy Preview for mobx failed.

Name Link
🔨 Latest commit e84f05e
🔍 Latest deploy log https://app.netlify.com/sites/mobx/deploys/633aa6dffe89f5000896186d

@codecov
Copy link

codecov bot commented Mar 8, 2022

Codecov Report

Merging #771 (e84f05e) into master (cea7013) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #771   +/-   ##
=======================================
  Coverage   98.90%   98.90%           
=======================================
  Files          55       55           
  Lines        1920     1923    +3     
=======================================
+ Hits         1899     1902    +3     
  Misses         21       21           
Flag Coverage Δ
flutter_mobx 100.00% <ø> (ø)
mobx 98.45% <100.00%> (+<0.01%) ⬆️
mobx_codegen 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mobx/lib/src/api/async/observable_future.dart 100.00% <100.00%> (ø)
mobx/lib/src/api/async/observable_stream.dart 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cea7013...e84f05e. Read the comment docs.

@amondnet amondnet marked this pull request as ready for review March 8, 2022 06:58
@amondnet amondnet changed the title feat: allow a custom equals parameter for ObservableStream Feat: Allow a custom equals parameter for ObservableStream Mar 8, 2022
@fzyzcjy fzyzcjy self-requested a review March 8, 2022 09:02
Copy link
Collaborator

@fzyzcjy fzyzcjy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@amondnet amondnet self-assigned this Mar 11, 2022
Copy link
Member

@pavanpodila pavanpodila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add to the docs

Copy link
Member

@pavanpodila pavanpodila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a changelog and version bump for the mobx package

@pavanpodila
Copy link
Member

Can you resolve conflicts pls before the merge. Also will need to bump up the version for publishing and add to changelog

Signed-off-by: Minsu Lee <amond@amond.net>
Signed-off-by: Minsu Lee <amond@amond.net>
@amondnet
Copy link
Collaborator Author

@pavanpodila Done.

@@ -332,7 +345,8 @@ class _ObservableStreamController<T> {
name: '$name.status'),
_valueType = Observable(_ValueType.value,
context: context, name: '$name.valueType'),
_data = Observable(initialValue, context: context, name: '$name.data') {
_data = Observable<dynamic>(initialValue,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not Observable<T?>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pavanpodila

Yes, I would like to use Observable<T?>, but Object type is assigned inside the _onError function.

  void _onError(Object error) {
    final actionInfo = _actions.startAction();
    try {
      _status.value = StreamStatus.active;
      _valueType.value = _ValueType.error;
      _data.value = error;
    } finally {
      _actions.endAction(actionInfo);
      _tryInsertInitialValue();
      _controller.addError(error);
    }
  }

@pavanpodila pavanpodila merged commit 87ba7d1 into mobxjs:master Oct 12, 2022
tlvenn pushed a commit to magelo-labs/mobx.dart_old that referenced this pull request Dec 31, 2022
* feat: allow a custom equals parameter for ObservableStream

Signed-off-by: Minsu Lee <amond@amond.net>

* feat: allow a custom equals parameter for ObservableStream

Signed-off-by: Minsu Lee <amond@amond.net>

Signed-off-by: Minsu Lee <amond@amond.net>
Co-authored-by: Pavan Podila <pavanpodila@users.noreply.github.com>
tlvenn pushed a commit to magelo-labs/mobx.dart_old that referenced this pull request Dec 31, 2022
* feat: allow a custom equals parameter for ObservableStream

Signed-off-by: Minsu Lee <amond@amond.net>

* feat: allow a custom equals parameter for ObservableStream

Signed-off-by: Minsu Lee <amond@amond.net>

Signed-off-by: Minsu Lee <amond@amond.net>
Co-authored-by: Pavan Podila <pavanpodila@users.noreply.github.com>
@amondnet
Copy link
Collaborator Author

@all-contributors please add @amondnet for doc

@amondnet amondnet deleted the custom-equals-stream branch May 10, 2023 03:27
@allcontributors
Copy link
Contributor

@amondnet

I've put up a pull request to add @amondnet! 🎉

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

Successfully merging this pull request may close these issues.

3 participants