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

Feat/use reactive #627

Merged
merged 7 commits into from
Oct 30, 2020
Merged

Feat/use reactive #627

merged 7 commits into from
Oct 30, 2020

Conversation

wen-haoming
Copy link
Contributor

@wen-haoming wen-haoming commented Sep 2, 2020

重构第一次提交的useReactive代码

Close #615

@CLAassistant
Copy link

CLAassistant commented Sep 2, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ brickspert
✅ wen-haoming
✅ awmleer
❌ 温浩明


温浩明 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@wen-haoming wen-haoming mentioned this pull request Sep 2, 2020
@awmleer
Copy link
Collaborator

awmleer commented Sep 2, 2020

hi~不要重复提交 PR,如果需要做补充修改的话,直接在原 PR 上 push 就可以了

@awmleer awmleer closed this Sep 2, 2020
@wen-haoming
Copy link
Contributor Author

hi~不要重复提交 PR,如果需要做补充修改的话,直接在原 PR 上 push 就可以了

呃... 我看到原来那个pr 不知道为啥远程仓库变成 unknown repository 😑 。。

@awmleer awmleer reopened this Sep 2, 2020

let state = useCreation(() => {
return observer(observerState, () => {
setObserverState({ ...observerState });
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里 {...observerState} 如果只是为了触发重渲染的话,其实可以直接用 useUpdate

既然是 reactive ,就不应该引用改变了。。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是呀 😅


import useCreation from '../useCreation';

function observer(initialVal, cb) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

所以现在是不依赖 vue reactive 了么。。直接用 Proxy 的话,不知道会不会在边界情况下不可靠?(例如数组的操作)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是呀,不依赖外面的包,直接使用proxy了,性能是最好的,至于数组的操作在改变原数组的时候能够触发,比如push pop之类,如果涉及slice肯定没效果的。

Copy link
Contributor Author

@wen-haoming wen-haoming Sep 6, 2020

Choose a reason for hiding this comment

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

不过这个hooks 最后返回的结果是一个可变对象,不过我觉得这样可能对于react本身不可变对象概念有点违背.. 比如如果直接使用useEffect去监听对象引用就不行了,需要监听值,不过我想到一个方法就是 let [ state, immutableObj ] = useReactive({count:0 }) 第一个state 是一个可变的操作对象,每次操作它, 第二个immutableObj 每次都是一个全新且完整的引用对象. 你觉得这样接口设计可以吗,而且我代码写好了 可以贴出来你看么😳

Choose a reason for hiding this comment

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

@wen-haoming

immutableObj怎么使用呢?

我思考过一个问题,所以翻到了这里看看有没有什么讨论。如果把 mutableState传给被React.memo的子组件,在子组件中改变它的值,那么子组件可能不会更新(因为proxy里面更新的是父组件,子组件被memo了,同时proxy的引用没有变)。

就算把ImmutableObj传递给子组件,可能会带来另一个问题,比如如果子组件用了const anotherMutable = useReactive(immutableObj)的话,然后子组件里面改变了anotherMutable,这时父组件就不会更新了,但是父组件使用的值是有变化的。

我没有想到好的解法,一个有点复杂的想法是这样的

  1. 维护一个对应的rootImmutableState。
  2. 每个proxy实例都提供一个 toImmutableState 的方法,获得rootImmutableState中与这个proxy对应的部分。
  3. 提供一个HOC,封装React.memo,比较的时候如果是proxy对象,就比较其 toImmutableState的值。
  4. 对于useMemo,useEffect等里面的的dependency的引用,需要手动toImmutableState

@awmleer awmleer self-assigned this Sep 20, 2020
@awmleer awmleer requested a review from brickspert October 27, 2020 03:47
@awmleer awmleer added the feature New feature or request label Oct 27, 2020
Copy link
Collaborator

@brickspert brickspert left a comment

Choose a reason for hiding this comment

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

LGTM

@awmleer awmleer merged commit b29b48a into alibaba:master Oct 30, 2020
@github-actions
Copy link

github-actions bot commented Oct 30, 2020

😭 Deploy PR Preview b29b48a failed. Build logs

🤖 By surge-preview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] useReactive & useMethods
5 participants