MichaelMarner / dart-redux-remote-devtools

Remote Devtools for Dart & Flutter
https://pub.dartlang.org/packages/redux_remote_devtools
MIT License
52 stars 10 forks source link

Let connect() method wait until there is an actual connection #16

Closed dennis-tra closed 5 years ago

dennis-tra commented 5 years ago

With this snippet I ran into a race condition

  var remoteDevtools = RemoteDevToolsMiddleware('localhost:8000');

  final store = DevToolsStore<AppState>(
    appReducer,
    initialState: AppState.initialState(),
    middleware: []
      ..add( my middlewares )
      ..add(remoteDevtools),
  );
  remoteDevtools.store = store;

  await remoteDevtools.connect();

  store.dispatch(Action1());
  store.dispatch(Action2());

after the await remoteDevtools.connect(); has resolved the connection is actually not yet established but it is in the starting state. The lead to my first dispatched actions not to appear in the Remote Dev Portal. The condition for forwarding actions is this.status == RemoteDevToolsStatus.started which was not the case yet.

This pull request waits until the remote dev tools have responded with START and then resolves the connect() functions.

MichaelMarner commented 5 years ago

LGTM. Would you be able to update the tests and make them pass? Looks like just updating the mocks to simulate receiving the START message from a server.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 90


Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/src/remote_devtools_middleware.dart 8 9 88.89%
<!-- Total: 8 9 88.89% -->
Totals Coverage Status
Change from base Build 85: 0.9%
Covered Lines: 88
Relevant Lines: 96

💛 - Coveralls
dennis-tra commented 5 years ago

@MichaelMarner I updated the tests 👍

MichaelMarner commented 5 years ago

Awesome, thanks for the contribution!

MichaelMarner commented 5 years ago

This is now live in 0.0.9 on pub.dev

dennis-tra commented 5 years ago

Perfect, thanks for merging :)