storage: solve hydration issue

[atk]

In order to simplify avoiding hydration issues, makePersisted now accepts isHydrated from the lifecycle package as an option to delay the initialization from storage until the parent is hydrated; since this way both server and client will have the same default on initial rendering, this will make them render the same, thus no longer causing hydration mismatch errors.

Another solution would be to sync the storage with the server and use makePersisted on the server, too.

This addresses #737

[changeset-bot]

🦋 Changeset detected

Latest commit: b3a785b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@solid-primitives/storage	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[thetarnav]

Why not use isHydrated (or it's implementation directly since it's just some "is hydrating" checks and onMount) in the primitive by default?
The primitives should be usable with ssr without needing to enable that support.

if (!isServer && sharedConfig.context) {/*delay init*/}
else {/*init immediately*/}

This is what all the primitives what read something from the dom like createWindowSize do. Even though window.innerWidth can be read immediately, it is delayed when hydrating.

[atk]

We do not know if the server and the client will have access to the same data on their storage. You could also using the sync API or cookieStorage to keep them in sync, so I do not want to make it a default, because then the delayed initialization on the client could introduce hydration mismatches if the fallback is hydrated.

[thetarnav]

Is there a way to ensure consistency automatically?
I don't mean sync.
Just a way for the server to "tell the client" to use a fallback or not?
Like this seems like something that the primitive should handle on it's own.
Maybe by using resources?
Which also have a onHydrated callback option that could be used to handle hydration.

[thetarnav]

Is there a way to ensure consistency automatically?

If this is out of scope of this pr, and you just want to introduce a temporary fix, then I don't mind.
But the api could be less convoluted imo.
For example as a option flag:

makePersisted(init, {useInitDuringHydration: true})

As opposed to telling users to pull another dependency.
It's not like isHydrated can have multiple implementations to allow for an interface.

[atk]

There is no way for us to know on either server or client what data the other side will have unless we either sync them manually or deliberately let them be out of sync.

It's not like isHydrated can have multiple implementations to allow for an interface

No, but one could also choose this to block initialization until any moment of your choice.

[atk]

OK, I switched to your suggested solution.

[thetarnav]

looks good
although there are some “isHydrated” leftovers in the comments and readme
also if you

No, but one could also choose this to block initialization until any moment of your choice.

If you think this valuable, delayInit could accept a function matching onMount type ({delayInit: onMount}), which would give the flexibility while keeping the api simple.
But yeah, no idea if that’s necessary.

[jcalfee]

The new delayInit name is great. I was thinking defer, but "delay" helps make this unique. Doesn't isHydrated happen before onMount though? If it isHyraded v. onMount makes any real difference, esp. re-rendering, I suggest to choose well if one always works or go back to the original PR with a function passed in but with the name changed to delayInit.

And fyi, good idea in your example @thetarnav, but I don't think onMount returns a value (I'm not on the latest version though). We might want to keep isHydrated in the docs at least.. I'm making some assumptions here though I don't know all the details about the difference in onMount and isHydrated.

function onMount(fn) {
  createEffect(() => untrack(fn));
}

[thetarnav]

@jcalfee

Doesn't isHydrated happen before onMount though?

No, isHydrated is pretty much onMount as a signal with a hydration check:
https://github.com/solidjs-community/solid-primitives/blob/main/packages/lifecycle/src/index.ts#L35

good idea in your example @thetarnav, but I don't think onMount returns a value

why would it return a value?

currently the logic is:

const initialize = () => {...}
if (options.deferInit) {
    onMount(initialize)
} else {
    initialize()
}

and my idea would look something like this

const initialize = () => {...}
if (options.deferInit) {
    options.deferInit(initialize)
} else {
    initialize()
}

[jcalfee]

I was referring to this:

If you think this valuable, delayInit could accept a function matching onMount type ({delayInit: onMount}),

If onMount did return a value that would be useful.

[atk]

options.deferInit(initialize) is somewhat of a foot gun IMO. If you don't read the instructions carefully, you'll easily break things. I think a boolean i option s fine for now. We can iterate if necessary.