Skip to content

Conversation

MrMugame
Copy link
Contributor

I don't want to offend anybody, but the code in the useEffect here, was really not it. This should also fix a bug, where you couldn't force white mode if your system is set to dark mode.
And maybe we should also add something to the docs about the use of the clientStore here, because this caused confusion multiple times now.

@MrMugame MrMugame marked this pull request as draft September 14, 2025 02:17
@MrMugame MrMugame marked this pull request as ready for review September 14, 2025 22:13
// otherwise leave it so the client can choose depending on the system
// settings
const darkMode = await clientStore.get("darkMode");
if (darkMode != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The != operator does not exist in my definition of JavaScript, surprised that lint even allows it 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I wasn't 100% sure what this would/could return here. Because if you follow the definition, some are typed to return null and some return undefined and there are some parts were typescript doesn't check

@zefhemel
Copy link
Collaborator

I have not put much thought into this, but I'm not sure we shouldn't deprecate clientStore altogether, since all our stores our now client-side anyway, maybe it should just be replaced with datastore everywhere? clientStore was always a bit weird because of the different key format (strings vs lists of strings).

Separate topic, I'm ok to merge this on its own.

@zefhemel zefhemel merged commit 440cd8d into silverbulletmd:main Sep 15, 2025
2 checks passed
@MrMugame
Copy link
Contributor Author

I have not put much thought into this, but I'm not sure we shouldn't deprecate clientStore altogether, since all our stores our now client-side anyway, maybe it should just be replaced with datastore everywhere? clientStore was always a bit weird because of the different key format (strings vs lists of strings).

I guess it uses datastore under the hood anyways. I'm not sure how many people use it, I use it in silversearch, just because it's a little easier to use than the datastore api.

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