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

Add an option to make BuiltReduxUiComponent "pure" #140

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/src/component_declaration/component_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component imple
@override
Future<T> getManagedDelayedFuture<T>(Duration duration, T callback()) =>
_getDisposableProxy().getManagedDelayedFuture<T>(duration, callback);

@override
ManagedDisposer getManagedDisposer(Disposer disposer) => _getDisposableProxy().getManagedDisposer(disposer);

Expand All @@ -309,7 +309,7 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component imple
{Function onError, void onDone(), bool cancelOnError}) =>
_getDisposableProxy().listenToStream(
stream, onData, onError: onError, onDone: onDone, cancelOnError: cancelOnError);

@override
Disposable manageAndReturnDisposable(Disposable disposable) =>
_getDisposableProxy().manageAndReturnDisposable(disposable);
Expand All @@ -327,7 +327,7 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component imple
@override
void manageDisposer(Disposer disposer) =>
_getDisposableProxy().manageDisposer(disposer);

@override
void manageStreamController(StreamController controller) =>
_getDisposableProxy().manageStreamController(controller);
Expand Down
38 changes: 34 additions & 4 deletions lib/src/experimental/built_redux_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,28 @@ abstract class BuiltReduxUiComponent<
@override
void componentWillMount() {
super.componentWillMount();
_isDirty = false;
_setUpSub();
}

@mustCallSuper
@override
void componentWillReceiveProps(Map nextProps) {
super.componentWillReceiveProps(nextProps);
_tearDownSub();
var tNextProps = typedPropsFactory(nextProps);

if (tNextProps.store != props.store) {
_tearDownSub();
_setUpSub(nextProps);
}
}

@mustCallSuper
@override
void componentWillUpdate(Map nextProps, Map nextState) {
// _storeSub will only be null when props get updated, not on every re-render.
if (_storeSub == null) _setUpSub(nextProps);
bool shouldComponentUpdate(Map nextProps, Map nextState) {
if (isPure) return _isDirty || typedPropsFactory(nextProps).store != props.store;

return true;
}

@mustCallSuper
Expand All @@ -85,6 +92,20 @@ abstract class BuiltReduxUiComponent<
_tearDownSub();
}

@mustCallSuper
@override
void redraw([callback()]) {
_isDirty = true;

super.redraw(() {
_isDirty = false;

if (callback != null) callback();
});
}

bool _isDirty;

Substate _connectedState;

/// The substate values of the redux store that this component subscribes to.
Expand Down Expand Up @@ -145,6 +166,15 @@ abstract class BuiltReduxUiComponent<
/// Related: [connectedState]
Substate connect(V state);

/// Whether the component should be a "pure" component.
///
/// A "pure" component will only re-render when [connectedState] is updated or [redraw] is called directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Or when the store changes, right?

Should there also be a note showing usage of this? For example:

/// To enable this functionality, override this getter in a subclass to return `true`.
///
///  When using this, do not override [redraw] or [shouldComponentUpdate].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

/// To enable this functionality, override this getter in a subclass to return `true`. When set to true it
/// is not recommended to override [redraw] or [shouldComponentUpdate].
///
/// Related: [shouldComponentUpdate], [redraw]
bool get isPure => false;

StreamSubscription _storeSub;

void _setUpSub([Map propsMap]) {
Expand Down
77 changes: 76 additions & 1 deletion test/over_react/experimental/redux_component_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import 'redux_component_test/test_reducer.dart';

part 'redux_component_test/default.dart';
part 'redux_component_test/connect.dart';
part 'redux_component_test/pure.dart';

void main() {
ReducerBuilder<BaseState, BaseStateBuilder> baseReducerBuilder;
Expand Down Expand Up @@ -95,6 +96,78 @@ void main() {
expect(component.numberOfRedraws, 1);
});

group('properly redraws when isPure is true', () {
test('when an action is triggered', () async {
var store = new Store<BaseState, BaseStateBuilder, BaseActions>(
baseReducerBuilder.build(),
baseState,
baseActions,
);
var jacket = mount<TestPureComponent>((TestPure()..store = store)());
TestPureComponent component = jacket.getDartInstance();

store.actions.trigger1();
await new Future.delayed(Duration.ZERO);
expect(component.numberOfRedraws, 1);
});

test('by not redrawing when other props change', () async {
var store = new Store<BaseState, BaseStateBuilder, BaseActions>(
baseReducerBuilder.build(),
baseState,
baseActions,
);
var jacket = mount<TestPureComponent>((TestPure()..store = store)());
TestPureComponent component = jacket.getDartInstance();

expect(component.numberOfRedraws, 0);

jacket.rerender((TestPure()
..store = store
..id = 'new id'
)());

expect(component.numberOfRedraws, 0);
});

test('by redrawing when store changes', () async {
var store = new Store<BaseState, BaseStateBuilder, BaseActions>(
baseReducerBuilder.build(),
baseState,
baseActions,
);
var updatedStore = new Store<BaseState, BaseStateBuilder, BaseActions>(
baseReducerBuilder.build(),
new BaseState(),
new BaseActions(),
);
var jacket = mount<TestPureComponent>((TestPure()..store = store)());
TestPureComponent component = jacket.getDartInstance();

expect(component.numberOfRedraws, 0);

jacket.rerender((TestPure()..store = updatedStore)());

expect(component.numberOfRedraws, 1);
});

test('when calling redraw', () {
var store = new Store<BaseState, BaseStateBuilder, BaseActions>(
baseReducerBuilder.build(),
baseState,
baseActions,
);
var jacket = mount<TestPureComponent>((TestPure()..store = store)());
TestPureComponent component = jacket.getDartInstance();

expect(component.numberOfRedraws, 0);

component.redraw();

expect(component.numberOfRedraws, 1);
});
});

test('updates subscriptions when new props are passed', () async {
var store = new Store<BaseState, BaseStateBuilder, BaseActions>(
baseReducerBuilder.build(),
Expand All @@ -115,9 +188,11 @@ void main() {

jacket.rerender((TestDefault()..store = updatedStore)());

expect(component.numberOfRedraws, 2);

updatedStore.actions.trigger1();
await new Future.delayed(Duration.ZERO);
expect(component.numberOfRedraws, 2);
expect(component.numberOfRedraws, 3);
});
});
}
Expand Down
11 changes: 5 additions & 6 deletions test/over_react/experimental/redux_component_test/connect.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@ class TestConnectComponent
int numberOfRedraws = 0;

@override
render() => Dom.div()(connectedState);
void componentDidUpdate(Map prevProps, Map prevState) {
numberOfRedraws++;
}

@override
int connect(BaseState state) => state.count1;
render() => Dom.div()(connectedState);

@override
void setState(_, [callback()]) {
numberOfRedraws++;
if (callback != null) callback();
}
int connect(BaseState state) => state.count1;
}
11 changes: 5 additions & 6 deletions test/over_react/experimental/redux_component_test/default.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@ class TestDefaultComponent extends BuiltReduxUiComponent<BaseState, BaseStateBui
int numberOfRedraws = 0;

@override
BaseState connect(BaseState state) => state;
void componentDidUpdate(Map prevProps, Map prevState) {
numberOfRedraws++;
}

@override
render() => Dom.div()();
BaseState connect(BaseState state) => state;

@override
void setState(_, [callback()]) {
numberOfRedraws++;
if (callback != null) callback();
}
render() => Dom.div()();
}
28 changes: 28 additions & 0 deletions test/over_react/experimental/redux_component_test/pure.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// ignore_for_file: deprecated_member_use

part of over_react.component_declaration.redux_component_test;

@Factory()
UiFactory<TestPureProps> TestPure;

@Props()
class TestPureProps extends BuiltReduxUiProps<BaseState, BaseStateBuilder, BaseActions> {}

@Component()
class TestPureComponent extends BuiltReduxUiComponent<BaseState, BaseStateBuilder, BaseActions, TestPureProps, BaseState> {
int numberOfRedraws = 0;

@override
void componentDidUpdate(Map prevProps, Map prevState) {
numberOfRedraws++;
}

@override
bool get isPure => true;

@override
BaseState connect(BaseState state) => state;

@override
render() => Dom.div()();
}