Skip to content

Conversation

@jquense
Copy link
Member

@jquense jquense commented Oct 22, 2019

No description provided.

@jquense jquense requested a review from taion October 22, 2019 18:20
@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

Merging #13 into master will increase coverage by 0.53%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
+ Coverage   60.89%   61.43%   +0.53%     
==========================================
  Files          28       29       +1     
  Lines         289      293       +4     
  Branches       65       66       +1     
==========================================
+ Hits          176      180       +4     
  Misses        113      113
Impacted Files Coverage Δ
src/useSet.ts 100% <ø> (ø) ⬆️
src/useSafeState.ts 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e024289...ed9097f. Read the comment docs.

@taion
Copy link
Member

taion commented Oct 22, 2019

eh, seems like in most cases you wouldn't want to wrap useState explicitly.

can we make higher-order hooks a thing?

@jquense
Copy link
Member Author

jquense commented Oct 22, 2019

this is more flexible i think, i didn't really want to figure out the typing for overloading a default useState implementation

@taion
Copy link
Member

taion commented Oct 22, 2019

wouldn't it just be typeof useState?

@jquense
Copy link
Member Author

jquense commented Oct 22, 2019

how would you disambiguate between useSafeState(someDefaultValue) and passing in the state object. I don't think i want to do higher order hooks, this seems nicer at still allows that if we want to wrap it in our app

@taion
Copy link
Member

taion commented Oct 22, 2019

const useSafeState = createUseWhatever(useState);

maybe?

@jquense
Copy link
Member Author

jquense commented Oct 23, 2019

that's more complex and more typing than this tho :P plus if you do it in render absentmindedly it's gonna fail badly. I don't know that i see the benefit over the version here?

@jquense jquense merged commit 88c53cb into master Oct 23, 2019
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.

3 participants