Skip to content

Conversation

@zombieJ
Copy link
Member

@zombieJ zombieJ commented Apr 13, 2022

follow up #405

close #405

@vercel
Copy link

vercel bot commented Apr 13, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/field-form/7gSzrozocseGzRSBtT4HUMa7gaaw
✅ Preview: https://field-form-git-usewatch-react-component.vercel.app

@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #413 (f6b27b6) into master (b8a15f5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #413   +/-   ##
=======================================
  Coverage   99.90%   99.90%           
=======================================
  Files          15       16    +1     
  Lines        1067     1111   +44     
  Branches      226      231    +5     
=======================================
+ Hits         1066     1110   +44     
  Misses          1        1           
Impacted Files Coverage Δ
src/FieldContext.ts 100.00% <ø> (ø)
src/index.tsx 100.00% <100.00%> (ø)
src/useForm.ts 99.76% <100.00%> (+0.01%) ⬆️
src/useWatch.ts 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

}
}

this.notifyWatch([namePath]);
Copy link
Member

Choose a reason for hiding this comment

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

这里的 notifyWatch 感觉位置不太对。。是不是该跟着 updateStore 后面?
或者有没有可能可以把 notifyWatch notifyObservers updateStore 合成一个方法?逻辑上应该是串联的

Copy link
Contributor

@crazyair crazyair Apr 13, 2022

Choose a reason for hiding this comment

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

本来可以把 notifyWatch 放到 notifyObservers 里,但是还有一点点不同,所以区分开了。不然也容易混乱

Copy link
Member Author

Choose a reason for hiding this comment

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

@MadCcc OK 不?~

Copy link
Member

Choose a reason for hiding this comment

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

感觉上问题不大,setState 相同 value 也不会触发 render

Copy link
Contributor

Choose a reason for hiding this comment

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

会有一次缓存问题(会多 render 一次),明天再看看

@zombieJ zombieJ merged commit f27229f into master Apr 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the useWatch branch April 13, 2022 14:54
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.

4 participants