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

3.0.0: Dart 2 Support #23

Merged
merged 1 commit into from
Mar 30, 2018
Merged

3.0.0: Dart 2 Support #23

merged 1 commit into from
Mar 30, 2018

Conversation

brianegan
Copy link
Collaborator

@brianegan brianegan commented Mar 29, 2018

We were having some type issues with Dart 2, and I found that we needed to change the implementation of Redux a bit. Overall, I actually think there might be an underlying bug with Dart 2, but this change simplifies the way our typed combinations work and provides good support for Dart 2.

Fixes #22

Changes:

  • Remove ReducerBinding, use TypedReducer
  • Remove combineTypedReducer. Use combineReducers with normal reducers & TypedReducers.
  • Remove MiddlewareBinding, use TypedMiddleware.
  • Remove combineTypedMiddleware -- no longer needed! Just create a normal List<Middleware<State>>!
  • Update docs for all of these cases

Copy link
Collaborator

@johnpryan johnpryan left a comment

Choose a reason for hiding this comment

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

Look good overall - just a question and a nitpick

Remove combineTypedMiddleware -- no longer needed! Just create a normal List<Middleware>

This is a nice improvement!

FLUTTER_BUILD_MODE=debug
FLUTTER_BUILD_DIR=build
SYMROOT=${SOURCE_ROOT}/../build/ios
FLUTTER_FRAMEWORK_DIR=/Users/phillywiggins/lab/flutter/bin/cache/artifacts/engine/ios
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure why flutter generates these guys, let's leave them out

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack, good call. Will do.

@@ -7,4 +7,3 @@ with_content_shell: false
dart_task:
- test: --platform vm
- dartanalyzer: --fatal-warnings lib
- dartfmt
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've noticed that the Dart 1 and Dart 2 formatters format slightly differently. Is that why this was taken out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, no way I could satisfy both for the time being. Would like to add it back in eventually.

@brianegan
Copy link
Collaborator Author

Appreciate the review! I'll remove those extra files and publish a new version tomorrow morning :)

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.

None yet

3 participants