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.

Sign up for GitHub

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

Jump to bottom

Breaking Change: Change default StoreGroup #174

Merged
merged 24 commits into from Apr 27, 2017
Merged

Breaking Change: Change default StoreGroup #174

merged 24 commits into from Apr 27, 2017

Conversation

azu
Copy link
Member

@azu azu commented Apr 22, 2017 •
edited

  • Make CQRSStoreGroup default
    • Rename CQRSStoreGroup to StoreGroup
  • Remove old StoreGroup and QueuedStoreGroup
  • Update all examples
  • Update flow type definition
  • Add Store#setState

fix #141

azu added 3 commits April 23, 2017 01:09
@azu
Breaking Change: Change default StoreGroup
c76f4ef 
- Make `CQRSStoreGroup` default
   - Rename `CQRSStoreGroup` to `StoreGroup`
- Remove old `StoreGroup` and `QueuedStoreGroup`
   -  Carve out this to module https://github.com/almin/legacy-store-group
@azu
test(StoreGroup): add test onChange arguments
3b62a46
@azu
refactor(example): migrate shopping-cart to almin 0.12
6138e54
@azu
Copy link
Member Author

azu commented Apr 23, 2017

Before QueuedStoreGroup:

  • AddItemToCartUseCase: 2~6ms

image

After: StoreGroup

  • AddItemToCartUseCase: 1ms ~ 8ms

2017-04-23 14-17-00

@azu
Copy link
Member Author

azu commented Apr 23, 2017 •
edited

Thnking points:

Which is default implementation of Store#shouldStateUpdate?

  • ===
  • shallowEqual

@azu
Copy link
Member Author

azu commented Apr 23, 2017

I think that this point should be resolved by checker.
I want to prefer to use shllowEqual.

azu
azu commented Apr 23, 2017
View reviewed changes
src/Store.ts
* Update own state property if needed.
* If `this.shouldStateUpdate(currentState, newState)` return true, update `this.state` property with `newState`.
*/
setState(newState: State): void {
Copy link
Member Author

@azu azu Apr 23, 2017 •
edited

Choose a reason for hiding this comment

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

add Store#setState.

The motivation is that improve immutability checker.
To direct modify Store#state prevent the check.

@azu
Copy link
Member Author

azu commented Apr 23, 2017 •
edited

Prevent Store#state = a;
use Store#setState insteadof it.
Related: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-direct-mutation-state.md

React will allow to directly assign in constructor and componentWillReceiveProps.

@azu azu merged commit 6565875 into master Apr 27, 2017
@azu azu deleted the cahnge-default-store-group branch April 27, 2017 23:32
@azu azu mentioned this pull request Apr 28, 2017
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Main StoreGroup
1 participant
@azu