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

Ej/fix persisted store #746

Merged
merged 11 commits into from
Aug 22, 2023
Merged

Ej/fix persisted store #746

merged 11 commits into from
Aug 22, 2023

Conversation

lightlii
Copy link
Contributor

Quick fix, reinstates async await syntax when getting an item from the store.

I've also made the config store consistent since zustand was stringifying the store upon save wheras by default the lib stores it as json which was resulting in configs that look like this:

{
	"experiments-flags": "{\"state\":{\"backgroundMaps\":false},\"version\":0}",
	"background-maps": "{\"state\":{\"mapStyle\":\"\"},\"version\":0}",
	"ui": "{\"state\":{\"tabIndex\":0},\"version\":0}",
	"state": {
		"maximize": false,
		"fullscreen": false,
		"defaultWidth": 1000,
		"defaultHeight": 800,
		"isMaximized": false,
		"show": false,
		"x": 2199,
		// etc.
	}
}

@lightlii lightlii requested a review from ErikSin August 17, 2023 17:44
Copy link
Contributor

Choose a reason for hiding this comment

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

based on the typescript typings, it looks like get item has to return a string, and I assume createJSONStorage then just turns it back to an object...its seems like it is working.

playing around with it, I also realize that getItem, setItem, and removeItem don't need jsdoc typings, those are encapasulated it typing of the 'storage'

Copy link
Contributor

@ErikSin ErikSin left a comment

Choose a reason for hiding this comment

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

Can you get rid of the unnecessary typing before you merge please and thank you

Comment on lines 24 to 28
if (typeof state === 'string') {
store.set(key, JSON.parse(state))
} else {
store.set(key, state)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

with some more testing, it seems like zustand will automatically create your object into a string (so you don't need to do this typecheck)

@ErikSin
Copy link
Contributor

ErikSin commented Aug 21, 2023

You can just get rid of the typings for getItem, setItem, and removeItem, and then you can merge

@lightlii lightlii merged commit 6769cd5 into master Aug 22, 2023
9 checks passed
@lightlii lightlii deleted the ej/fix-persisted-store branch August 22, 2023 08:49
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.

2 participants