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

feat: new hooks useLocalStorageValue and useSessionStorageValue #43

Merged
merged 10 commits into from
May 16, 2021
Merged

feat: new hooks useLocalStorageValue and useSessionStorageValue #43

merged 10 commits into from
May 16, 2021

Conversation

xobotyi
Copy link
Contributor

@xobotyi xobotyi commented May 6, 2021 •
edited
Loading

What new hook does?

Add new hooks to manage localStorage and sessionStorage keys

Checklist

  • Have you read contribution guideline?
  • Does the code have comments in hard-to-understand areas?
  • Is there an existing issue for this PR?
  • Have the files been linted and formatted?
  • Have the docs been updated?
  • Have the tests been added to cover new hook?
  • Have you run the tests locally to confirm they pass?

@codecov
Copy link

codecov bot commented May 6, 2021 •
edited
Loading

Codecov Report

Merging #43 (c0ce8b1) into master (7d5fd03) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            master       #43    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           20        23     +3     
  Lines          163       282   +119     
  Branches        22        55    +33     
==========================================
+ Hits           163       282   +119
Impacted Files Coverage Δ
src/util/const.ts 100.00% <ø> (ø)
src/index.ts 100.00% <100.00%> (ø)
src/useLocalStorageValue.ts 100.00% <100.00%> (ø)
src/useSessionStorageValue.ts 100.00% <100.00%> (ø)
src/useStorageValue.ts 100.00% <100.00%> (ø)

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 7d5fd03...c0ce8b1. Read the comment docs.

JoeDuncko
JoeDuncko approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@JoeDuncko JoeDuncko left a comment

Choose a reason for hiding this comment

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

This is a lot of code to code review, so I may have missed something, but it looks good besides a few typos.

Question: Does this implementation sync multiple instances of useLocalStorageValue that use the same key? I know that was a common "problem" people had with react-use's implementation.

stories/SideEffects/useLocalStorageValue.stories.tsx Outdated Show resolved Hide resolved
stories/SideEffects/useSessionStorageValue.stories.tsx Outdated Show resolved Hide resolved
stories/SideEffects/useLocalStorageValue.story.mdx Outdated Show resolved Hide resolved
stories/SideEffects/useSessionStorageValue.story.mdx Outdated Show resolved Hide resolved
stories/SideEffects/useSessionStorageValue.stories.tsx Outdated Show resolved Hide resolved
@xobotyi
Copy link
Contributor Author

xobotyi commented May 7, 2021

Not yet but it will, as for now it syncs between tabs. I'll try to manage in-tab sync don't know how yet.

@JoeDuncko
Copy link
Contributor

Not yet but it will, as for now it syncs between tabs. I'll try to manage in-tab sync don't know how yet.

Should we make an issue for that?

@xobotyi
Copy link
Contributor Author

xobotyi commented May 7, 2021

No need, unless will merge this implementation

@JoeDuncko
Copy link
Contributor

Ah, didn't realize this was still in progress

@xobotyi xobotyi changed the title feat: new hooks useStorageValue, useLocalStorageValue feat: new hooks useStorageValue, useLocalStorageValue and useSessionStorageValue May 8, 2021
@xobotyi xobotyi changed the title feat: new hooks useStorageValue, useLocalStorageValue and useSessionStorageValue feat: new hooks useLocalStorageValue and useSessionStorageValue May 15, 2021
@xobotyi xobotyi marked this pull request as ready for review May 15, 2021 15:15
@xobotyi xobotyi requested a review from JoeDuncko May 15, 2021 15:15
@xobotyi
Copy link
Contributor Author

xobotyi commented May 15, 2021 •
edited
Loading

@JoeDuncko finally finished! it was a tough one.
Much simpler codebase now (but with the cost of functionality, synchronisation between hooks requires invariance in terms of serialization)

This was referenced May 15, 2021
Open
Open
Open
@xobotyi xobotyi added the 🎂 new hook New hook added label May 15, 2021
@xobotyi xobotyi merged commit f02e8ea into master May 16, 2021
@xobotyi xobotyi deleted the useStorageValue branch May 16, 2021 11:48
github-actions bot pushed a commit that referenced this pull request May 16, 2021
@semantic-release-bot
chore(release): 1.19.0 [skip ci]
ad7d3bd 
# [1.19.0](v1.18.0...v1.19.0) (2021-05-16)

### Features

* new hooks `useLocalStorageValue` and `useSessionStorageValue` ([#43](#43)) ([f02e8ea](f02e8ea))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 1.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeDuncko JoeDuncko Awaiting requested review from JoeDuncko

Assignees
No one assigned
Labels
🎂 new hook New hook added released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xobotyi @JoeDuncko