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

Hooks: useStorage hook multi tab awareness #2887

Closed
anud12 opened this issue May 13, 2022 · 5 comments · Fixed by #2888
Closed

Hooks: useStorage hook multi tab awareness #2887

anud12 opened this issue May 13, 2022 · 5 comments · Fixed by #2888
Assignees
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Milestone

Comments

@anud12
Copy link

anud12 commented May 13, 2022

Describe the feature you would like to see added

The current implementation for useStorage isn't aware of the storage event.

Is your feature request related to a problem?

Due to the naming of the hook I expected it to react on changes originating from multiple tabs.

Describe the solution you'd like

The way that i envisioned this functionality is to have 2 hooks rather than 1 useSessionStorage and useLocalStorage and that would've freed the last parameter to be a flag of enabling the reactivity.

Sadly with the current implementation without breaking changes the constraints are high on where to add the flag, if the goal in using it should be succinct.

Describe alternatives you have considered

No response

Additional context

No response

@anud12 anud12 added the Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add label May 13, 2022
@melloware melloware changed the title useStorage hook multi tab awareness Hook: useStorage hook multi tab awareness May 13, 2022
@melloware
Copy link
Member

@anud12 I think this can still be done let me take a look.

@melloware
Copy link
Member

Please review my PR: #2888

It now listens to other tabs and there are two new hooks useLocalStorage and useSessionStorage that are just convenience hook wrappers.

@melloware melloware changed the title Hook: useStorage hook multi tab awareness Hooks: useStorage hook multi tab awareness May 13, 2022
@anud12
Copy link
Author

anud12 commented May 15, 2022

Seems good.

Another small enhancement (but this is as pedantic as the convenience hooks) is to alter the definition a bit.
From

useStorage<S>(initialValue: S, key: string, storage?: StorageType): [S, React.Dispatch<React.SetStateAction<S>>]

To:

useStorage<S, K extends string = string>(initialValue: S, key: K, storage?: StorageType): [S, React.Dispatch<React.SetStateAction<S>>]

Because with that I can use the hook

type ValidKeyOptions = "key1" | "key2"
const [state, setState] = useStoarge<MyObject, ValidkeyOptions>({name: ""}, "key2");

This will ease the maintenance and refactoring because when i change the valid key options typescript would highlight if and where the key was used.

melloware added a commit to melloware/primereact that referenced this issue May 15, 2022
@melloware
Copy link
Member

I like that change. Just pushed it to my PR.

@melloware
Copy link
Member

Thanks for your great feedback and improvement. Since no PR component is using this hook yet this change is safe to make for 8.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants