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

Release: 1.0.0 Beta #59

Merged
merged 35 commits into from
Feb 16, 2020
Merged

Release: 1.0.0 Beta #59

merged 35 commits into from
Feb 16, 2020

Conversation

alvinhui
Copy link
Member

@alvinhui alvinhui commented Feb 14, 2020

使用示例

https://github.com/ice-lab/icestore/tree/release/1.0.0-beta/examples/todos

能力表

icestore
同步 Action O
异步 Action O
Class 组件 O
Hooks 组件 O
异步状态 O
中间件/插件 X
中心化 O
状态联动 O
SSR O
有无 Provider O
开发者工具 X
懒加载模型 O
Concurrent-Mode O

API

模型定义

// src/models/count.ts
export default  {
  state: {
    count: 0
  },
  reducers: {
    addBy(prevState, payload) => {...prevState, count: prevState.count + payload}
  },
  effects: {
    async addByAsync(payload, state, actions) {
      await delay(1000)

      // 调用本模型或其他模型的 action
      actions.count.addBy(payload)
    }
  }
}

模型注册

// src/store.ts
import { createStore } from '@ice/store';
import count from './models/count';

export default createStore({ count });

绑定视图

import store from './src/store';

const { Provider } = store;
ReactDOM.render(<Provider><App/></Provider>, rootElement);

模型消费

import store from './src/store';

function TodoAdd() {
  const [state, actions] = useModel('todos');

  // 只调用方法不消费
  const actions = useModelAction('todos');
  // actions.addByAsync

   // 消费模型
  const state = useModelState('todos');
  // state.count

  // 异步 actions 的状态
  const effectState = useModelEffectState('todos');
  // effectState.addByAsync.isLoading;
}

问题和讨论点

  1. 模型的定义是否需要区分 effects 和 reducers;

  2. 当前在 effect 调用本模型的 reducers 不是很友好,有几种解决方式:

    1. 添加 namespace 字段;
    2. 参数 actions 变成 getActions ,当调用该方法没有传参时,返回当前模型的 actions;
    3. this.actionName
  3. 由于 effectState 和 modelState 是分开的,所以会多一次将 isLoaing 设置为 false 的 setState 的触发。

@alvinhui alvinhui requested a review from chenbin92 February 14, 2020 08:54
@alvinhui alvinhui self-assigned this Feb 14, 2020
@chenbin92
Copy link
Contributor

chenbin92 commented Feb 14, 2020

模型的定义是否需要区分 effects 和 reducers;

不用区分就是下面这样是吧?

在 redux 之类的概念了,reducers 的概念类似 Array.prototype.reduce(reducer, ?initialValue) 主要还是体现函数式编程的概念,reducers 和 effects 形成一对对比,本质还是从 action => reducer => newState 这样的流程。我个人理解从现在的 API 设计和实现来说区分是合理的。不区分的收益点是什么

// src/models/count.ts
export default  {
  state: {
    count: 0
  },
  addBy: (prevState, payload) => {...prevState, count: prevState.count + payload}, 
  addByAsync:  async (payload, state, actions) =>  {
     await delay(1000)

     // 调用本模型或其他模型的 action
     actions.count.addBy(payload)
  }
}

@chenbin92
Copy link
Contributor

当前在 effect 调用本模型的 reducers 不是很友好,有几种解决方式:
添加 namespace 字段;
参数 actions 变成 getActions ,当调用该方法没有传参时,返回当前模型的 actions;
this.actionName

不知道你说的很不友好是指的什么,这里我觉得有一点不友好的是 actions.count.addBy(payload) 这一句,按照你上面的示例应该是要先 createStore({ count }); 在能调 actions.count.xxx 这样吧?也就是说在定义 store 的时候需要知道 modelName,但是 store 里并没有 modelName 的概念,会不会有点奇怪?

// src/models/count.ts
export default  {
  // 你说的 namespace 是指这样吧?
  namespace: 'count'

  effects: {
    async addByAsync(payload, state, actions) {
      await delay(1000)

      // 调用本模型或其他模型的 action
      actions.count.addBy(payload)
    }
  }
}

@chenbin92
Copy link
Contributor

由于 effectState 和 modelState 是分开的,所以会多一次将 isLoaing 设置为 false 的 setState 的触发。

这个需要看下代码实现,暂时可以作为后续的优化点迭代?

@alvinhui
Copy link
Member Author

alvinhui commented Feb 14, 2020

当前在 effect 调用本模型的 reducers 不是很友好,有几种解决方式:
添加 namespace 字段;
参数 actions 变成 getActions ,当调用该方法没有传参时,返回当前模型的 actions;
this.actionName

不知道你说的很不友好是指的什么,这里我觉得有一点不友好的是 actions.count.addBy(payload) 这一句,按照你上面的示例应该是要先 createStore({ count }); 在能调 actions.count.xxx 这样吧?也就是说在定义 store 的时候需要知道 modelName,但是 store 里并没有 modelName 的概念,会不会有点奇怪?

// src/models/count.ts
export default  {
  // 你说的 namespace 是指这样吧?
  namespace: 'count'

  effects: {
    async addByAsync(payload, state, actions) {
      await delay(1000)

      // 调用本模型或其他模型的 action
      actions.count.addBy(payload)
    }
  }
}

是的,就是「在定义 store 的时候需要知道 modelName,但是 store 里并没有 modelName 的概念」,这点不友好。

是的,namespace 就是你示例中这样。我个人比较倾向于

async addByAsync(payload, state, getActions) {
    getActions(); // 获取本模型的 actions
    geActions('user') // 获取其他模型的 actions
}

这样处理。

@alvinhui
Copy link
Member Author

由于 effectState 和 modelState 是分开的,所以会多一次将 isLoaing 设置为 false 的 setState 的触发。

这个需要看下代码实现,暂时可以作为后续的优化点迭代?

+1

effectState 和 modelState 分开我觉得有一些好处。

比方说在未 useEffectState 的组件里,是不会订阅异步 action 的 loading 变化的。

@chenbin92
Copy link
Contributor

chenbin92 commented Feb 14, 2020

当前在 effect 调用本模型的 reducers 不是很友好,有几种解决方式:
添加 namespace 字段;
参数 actions 变成 getActions ,当调用该方法没有传参时,返回当前模型的 actions;
this.actionName

不知道你说的很不友好是指的什么,这里我觉得有一点不友好的是 actions.count.addBy(payload) 这一句,按照你上面的示例应该是要先 createStore({ count }); 在能调 actions.count.xxx 这样吧?也就是说在定义 store 的时候需要知道 modelName,但是 store 里并没有 modelName 的概念,会不会有点奇怪?

// src/models/count.ts
export default  {
  // 你说的 namespace 是指这样吧?
  namespace: 'count'

  effects: {
    async addByAsync(payload, state, actions) {
      await delay(1000)

      // 调用本模型或其他模型的 action
      actions.count.addBy(payload)
    }
  }
}

是的,就是「在定义 store 的时候需要知道 modelName,但是 store 里并没有 modelName 的概念」,这点不友好。

是的,namespace 就是你示例中这样。我个人比较倾向于

async addByAsync(payload, state, getActions) {
    getActions(); // 获取本模型的 actions
    geActions('user') // 获取其他模型的 actions
}

这样处理。

那完整的定义 store 应该就是下面这样?

// src/models/count.ts
export default  {
  namespace: 'count'

 reducers: {
    addBy(prevState, payload) => {...prevState, count: prevState.count + payload}
  },
  effects: {
    async addByAsync(payload, state, getActions) {
     // 每次定义 store 都要调用这一步,我感觉有点多余
     // 我偏向这种 actions.count.xxx 
      const actions  = getActions() // or getActions('user')
      await delay(1000)

      // 这里有点没看懂
      // 1. 这里的 state 表示什么?没看到被使用?
     //  2. 这里的 payload 我理解 view 调用时的参数,然后直接传给 payload 了? 正常应该是这里异步返回的值是吧?
      actions.count.addBy(payload)
    }
  }

src/createContainer.tsx Outdated Show resolved Hide resolved
src/createContainer.tsx Show resolved Hide resolved
src/createContainer.tsx Show resolved Hide resolved
src/createContainer.tsx Show resolved Hide resolved
src/createStore.tsx Outdated Show resolved Hide resolved
(state) => ({ ...state, subTitle: 'SubTitle' }),
(actions) => actions,
(effectsState) => effectsState,
)(TodoList);
Copy link
Contributor

Choose a reason for hiding this comment

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

如果这里要用 user Model 要怎么写

Copy link
Member Author

Choose a reason for hiding this comment

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

components/Todos.tsx 下有 useModel 的写法,如果是 Class 则必须是用 connect。

Copy link
Member Author

Choose a reason for hiding this comment

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

提供 withModel,withModels 两个 API ,后者可以获取多个 models

Copy link
Member Author

Choose a reason for hiding this comment

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

done


const { connect } = store;

class TodoList extends Component<any> {
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. 示例我觉得保持一致写法就可以了。都用 Function Component
  2. 如果要演示 connect 单独写个非常简单的 example 或者文档透出就行了

Copy link
Member Author

Choose a reason for hiding this comment

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

单独也一个 example 感觉成本太高了。希望是只有一个 example 然后覆盖所有用法?

Copy link
Member Author

Choose a reason for hiding this comment

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

默认用 FC component,然后有一个 Class component 的示例

Copy link
Member Author

Choose a reason for hiding this comment

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

done

examples/todos/src/index.tsx Outdated Show resolved Hide resolved
}

function useModelEffectState(namespace: string) {
const [, , , useModelEffectState ] = modelContainers[namespace];
Copy link
Contributor

Choose a reason for hiding this comment

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

话说这种写法不如对象 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

我已经爱上这种写法了😆

@alvinhui
Copy link
Member Author

alvinhui commented Feb 14, 2020

模型的定义是否需要区分 effects 和 reducers;

不用区分就是下面这样是吧?

在 redux 之类的概念了,reducers 的概念类似 Array.prototype.reduce(reducer, ?initialValue) 主要还是体现函数式编程的概念,reducers 和 effects 形成一对对比,本质还是从 action => reducer => newState 这样的流程。我个人理解从现在的 API 设计和实现来说区分是合理的。不区分的收益点是什么

// src/models/count.ts
export default  {
  state: {
    count: 0
  },
  addBy: (prevState, payload) => {...prevState, count: prevState.count + payload}, 
  addByAsync:  async (payload, state, actions) =>  {
     await delay(1000)

     // 调用本模型或其他模型的 action
     actions.count.addBy(payload)
  }
}

纯对象的话,好处就是:

  • TS 支持方便一些(毕竟是一个对象了);
  • 不会有重命名的问题(现在如果 reducer 和 effect 重名,后者覆盖前者);
  • 减少一些样板代码(不需要又用 effects 去调 reducer 了,注意现在 effects 内是没法直接更新 state 的);
  • 概念更多了,Function 入参和出参的不一致性;
  • 定义时是 reducer 和 effects,使用时又不需要做区分,这里对于用户理解来说有一些成本在。

redux style 的好处就是:

  • 与 React Hooks 的心智一致;
  • 内部实现没有魔法,也更简单一些,reducer 就是 setState 的 parm,effects 才需要延迟执行;
  • 有拓展的空间,例如未来加个字段啥的?

结论:

export default {
    state: {},
    actions: {
      add(){},
      async addAsync() {}
    }
}

@alvinhui
Copy link
Member Author

// todos model

// 1
const actions  = getActions() 
actions.add();
const userActions = getActions('user');
userActions.foo();

// 2
actions.todos.add(); // namespace
actions.user.foo();

// 3
// payload, state, actions, globalActions
actions.add,
globalActions.user.foo
globalActions.product.foo

// 4 
this.add
actions.user.foo

选择第三种。

@alvinhui
Copy link
Member Author

done.

];
}

function useModels(namespaces: string[], createMapState?, createMapActions?, createMapActionsState?) {
Copy link
Contributor

Choose a reason for hiding this comment

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

不符合 Hooks 的规则

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@chenbin92
Copy link
Contributor

chenbin92 commented Feb 16, 2020

@alvinhui +1

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.

2 participants