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

Rework useStorageValue to more simple and robust variant #960

Merged
merged 22 commits into from
Nov 3, 2022

Conversation

xobotyi
Copy link
Contributor

@xobotyi xobotyi commented Oct 11, 2022

New hook has less options, now it is impossible to make hook isolated, all hooks listening same key are synchronised.

Also hook return signature changed to object instead of array, as it provides more obvious API.

close #959
fix #451

BREAKING CHANGE: new implementation brings different API.
It is not backward compatible!
@xobotyi xobotyi self-assigned this Oct 11, 2022
@xobotyi xobotyi marked this pull request as draft October 11, 2022 09:07
@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #960 (b5b776e) into master (532cc41) will decrease coverage by 0.38%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##            master     #960      +/-   ##
===========================================
- Coverage   100.00%   99.61%   -0.39%     
===========================================
  Files           61       61              
  Lines         1065     1039      -26     
  Branches       184      165      -19     
===========================================
- Hits          1065     1035      -30     
- Misses           0        2       +2     
- Partials         0        2       +2     
Impacted Files Coverage Δ
src/useLocalStorageValue/useLocalStorageValue.ts 77.77% <100.00%> (-22.23%) ⬇️
...c/useSessionStorageValue/useSessionStorageValue.ts 77.77% <100.00%> (-22.23%) ⬇️
src/useStorageValue/useStorageValue.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xobotyi
Copy link
Contributor Author

xobotyi commented Oct 15, 2022

@ArttuOll could you please finish this one?

I think only docs left for now, the code itself and tests are already done.

@ArttuOll
Copy link
Contributor

Sure! I'll take a look over the next few days.

# [16.0.0](v15.1.0...v16.0.0) (2022-10-09)

### Styles

* remove I prefix from types and interfaces ([c2a1ff4](c2a1ff4))

### BREAKING CHANGES

* `I` prefix removed from all types having it.
)

Bumps [jest-environment-jsdom](https://github.com/facebook/jest/tree/HEAD/packages/jest-environment-jsdom) from 29.1.2 to 29.2.0.
- [Release notes](https://github.com/facebook/jest/releases)
- [Changelog](https://github.com/facebook/jest/blob/main/CHANGELOG.md)
- [Commits](https://github.com/facebook/jest/commits/v29.2.0/packages/jest-environment-jsdom)

---
updated-dependencies:
- dependency-name: jest-environment-jsdom
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [jest](https://github.com/facebook/jest/tree/HEAD/packages/jest) from 29.1.2 to 29.2.0.
- [Release notes](https://github.com/facebook/jest/releases)
- [Changelog](https://github.com/facebook/jest/blob/main/CHANGELOG.md)
- [Commits](https://github.com/facebook/jest/commits/v29.2.0/packages/jest)

---
updated-dependencies:
- dependency-name: jest
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… useDeepCompareEffect (#964)

Added missing example for useDeepCompareEffect.

fix #582
…967)

Bumps [@react-hookz/eslint-config](https://github.com/react-hookz/eslint-config) from 1.7.3 to 1.7.4.
- [Release notes](https://github.com/react-hookz/eslint-config/releases)
- [Changelog](https://github.com/react-hookz/eslint-config/blob/master/CHANGELOG.md)
- [Commits](react-hookz/eslint-config@v1.7.3...v1.7.4)

---
updated-dependencies:
- dependency-name: "@react-hookz/eslint-config"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [jest](https://github.com/facebook/jest/tree/HEAD/packages/jest) and [@types/jest](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/jest). These dependencies needed to be updated together.

Updates `jest` from 29.2.0 to 29.2.1
- [Release notes](https://github.com/facebook/jest/releases)
- [Changelog](https://github.com/facebook/jest/blob/main/CHANGELOG.md)
- [Commits](https://github.com/facebook/jest/commits/v29.2.1/packages/jest)

Updates `@types/jest` from 29.1.2 to 29.2.0
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/jest)

---
updated-dependencies:
- dependency-name: jest
  dependency-type: direct:development
  update-type: version-update:semver-patch
- dependency-name: "@types/jest"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Bumps [jest-environment-jsdom](https://github.com/facebook/jest/tree/HEAD/packages/jest-environment-jsdom) from 29.2.0 to 29.2.1.
- [Release notes](https://github.com/facebook/jest/releases)
- [Changelog](https://github.com/facebook/jest/blob/main/CHANGELOG.md)
- [Commits](https://github.com/facebook/jest/commits/v29.2.1/packages/jest-environment-jsdom)

---
updated-dependencies:
- dependency-name: jest-environment-jsdom
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@storybook/storybook-deployer](https://github.com/storybooks/storybook-deployer) from 2.8.12 to 2.8.16.
- [Release notes](https://github.com/storybooks/storybook-deployer/releases)
- [Changelog](https://github.com/storybookjs/storybook-deployer/blob/master/CHANGELOG.md)
- [Commits](storybook-eol/storybook-deployer@v2.8.12...v2.8.16)

---
updated-dependencies:
- dependency-name: "@storybook/storybook-deployer"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.19.3 to 7.19.6.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.19.6/packages/babel-core)

---
updated-dependencies:
- dependency-name: "@babel/core"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Conflicts:
#	CHANGELOG.md
#	package.json
@xobotyi xobotyi marked this pull request as ready for review October 24, 2022 09:32
@xobotyi
Copy link
Contributor Author

xobotyi commented Oct 24, 2022

@JoeDuncko @ArttuOll please review PR as a whole.

Copy link
Contributor

@ArttuOll ArttuOll left a comment

Choose a reason for hiding this comment

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

The hook seems to work as expected.

One thing I noticed was that the code example does not show the usage of the hook itself; it just shows the rendered HTML. I could not figure out how to make it show the hook though.

I commented on a couple spots where the exhaustive-deps lint rule seemed to be unnecessarily disabled, which I personally dislike a lot, but it does not really affect the functionality of the hook here.

src/useStorageValue/useStorageValue.ts Show resolved Hide resolved
src/useStorageValue/useStorageValue.ts Show resolved Hide resolved
@xobotyi
Copy link
Contributor Author

xobotyi commented Oct 25, 2022

There is barely possible example of useStorageValue hook, also it is not exported at all.

As for useSessionStorage/LocalStorage - they're pretty functional, based on inputs, showing that hooks are synced across the page and, in case of LocalStoage - across tabs

@ArttuOll
Copy link
Contributor

ArttuOll commented Oct 25, 2022

There is barely possible example of useStorageValue hook, also it is not exported at all.

As for useSessionStorage/LocalStorage - they're pretty functional, based on inputs, showing that hooks are synced across the page and, in case of LocalStoage - across tabs

Sorry, I think I was a bit unclear. I meant that the examples of useLocalStorageValue and useSessionStorageValue do not show the usage of the hook in their code examples (when you press the "show code" button in Storybook). The examples are good, but you cannot see the usage of the hook in the example code in Storybook, which I think would be helpful.

@xobotyi
Copy link
Contributor Author

xobotyi commented Oct 30, 2022

Have no idea how to make storybook to better handle code preview =(
Also have an idea of migrating to docosaurus since storybook caused lots of issues recently, but it is another discussion =)

@ArttuOll
Copy link
Contributor

We have other examples also where this is a problem. I was thinking of going over the docs some time and doing little improvements like this. I'll worry about this then.

@ArttuOll ArttuOll self-requested a review October 30, 2022 18:24
@xobotyi xobotyi merged commit 7bcc385 into master Nov 3, 2022
@xobotyi xobotyi deleted the rework-useStorageValue branch November 3, 2022 13:03
github-actions bot pushed a commit that referenced this pull request Nov 3, 2022
# [17.0.0](v16.1.0...v17.0.0) (2022-11-03)

* Rework `useStorageValue` to more simple and robust variant (#960) ([7bcc385](7bcc385)), closes [#960](#960)

### BREAKING CHANGES

* new implementation brings different API.
It is not backward compatible!

Co-authored-by: Arttu Olli <arttuoll@tutanota.com>
@xobotyi
Copy link
Contributor Author

xobotyi commented Nov 3, 2022

🎉 This PR is included in version 17.0.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
Labels
Development

Successfully merging this pull request may close these issues.

useSessionStorageValue same key, different value
4 participants