feat: implement useSsrState hook

[xobotyi]

What new hook does?

Provides stable value that other hooks can use to determine that applocation is in SSR mode.

Sad enough there are no reliable markers that application is hydrated after SSR - therefore we have to introduce own context.

Checklist

• Have you read contribution guideline?
• Does the code have comments in hard-to-understand areas?
• Is there an existing issue for this PR?
  • link issue here
• Have the files been linted and formatted?
• Have the docs been updated?
• Have the tests been added to cover new hook?
• Have you run the tests locally to confirm they pass?

[codecov]

Codecov Report

Merging #1023 (b237521) into master (1c1d098) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1023   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files          61       62    +1     
  Lines        1041     1048    +7     
  Branches      163      163           
=======================================
+ Hits         1037     1044    +7     
  Misses          2        2           
  Partials        2        2           

Impacted Files	Coverage Δ
src/index.ts	100.00% <100.00%> (ø)
src/useSsrState/useSsrState.tsx	100.00% <100.00%> (ø)

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

[ArttuOll]

I will have time to review this on Friday.

I have two questions about this:

1. Will there be an option to use the hooks as usual with the initializeWithValue prop? If a user uses only a couple of our hooks in their app, it might be annoying for them to have to wrap their whole application in a context provider and they would probably rather use the initializeWithValue option.
2. Partly related to the previous question: what's the plan after this is merged? Will we refactor all isomorphic hooks to use this under the hood?

[xobotyi]

1. Id rather remove all options.
   This hook can be reworked to use variables instead of context, to avoid context bloating.
2. Yes the plan is to rework all currently isomorphic hooks and make major release.

[xobotyi]

But there will be an issue with typing, that I have no idea how to overcome

[ArttuOll]

Looks good to me. So basically this is a boolean context that the isomorphic hooks read to know whether to initialize with a value or not.

One question though: should we wait until all isomorphic hooks are refactored to use this hook before merging this? Currently this hook does nothing, even though the docs say that it affects other hooks of ours.

The users could use this hook to build their own isomorphic hooks though. Maybe that should be mentioned in the docs too?

[xobotyi]

Before merge we have to figure out how to make proper types for isomorphic hooks. As for now - it is pretty easy to alter return types basing of function parameter, but in case of switching to this hook - it is pretty much impossible to type it properly (at leas in my mind, research needed)

[kylemh]

This is very tricky. I wanted throw a suggestion out there:

What if we split the react-hookz/web to have client-only and SSR-ready hooks? The type safety would be automatic and developing on the hooks from within this repo may be a bit simpler because the contexts are separated by a folder.

Basically, thinking back to #1000 there'd be a useMediaQuery exported from @react-hookz/web/client-rendered and from @react-hookz/web/server-rendered. It would also let people who deal with hybrid applications leverage linting to prohibit the wrong folder from being imported within each context.

[xobotyi]

It will simply double the amount of supported code and will lead to issues when developer uses different hooks in different places. Linter for this case is weak excuse.

[xobotyi]

What i'm thinking now - we can offer type extension for SSR users, and basing on that extension all types for SSR-sensitive hooks will switch to SSR-compatibility mode.
Guess it is possible just dont yet know how.

[kylemh]

does something like this help? https://twitter.com/benlesh/status/1610298056540397568?s=46&t=Ar1ixjc9BhyFJS15ooM1Fg

[xobotyi]

Nah, it is not a problem to set state - problem is in typings of hooks return.
Now it works basing off parameters value, with this hook i dont yen know how exactly solve type alternation, therefore this hook is not merged yet

[kylemh]

I got hung up on the idea that we could conditionally use a different hook for #1000 and maybe define the initial value within only useEffect, but that doesn't solve the issue you mentioned earlier of strict mode complaining about different values on subsequent renders.

Ignore me.

[xobotyi]

Just as an idea:
what SSR users think about the solution i18n uses: https://react.i18next.com/latest/typescript

So basically it will require to redeclare some types, along with such hook (that i'm planning to migrate from conext to globally stored variable) -thus youll have a single change to make all compatible hooks to switch to SSR state

[kylemh]

That still only adjusts the types, no? Won't all client-only users suffer from an extra render and bundled code?

[xobotyi]

Thre is basically no difference, afaik, since i don believe current useStorageValue has SSR-compliant code eliminated

[kylemh]

hmmm the types don't solve the bigger DX problem which is that depending on your apps context, you'd need to provide arguments to certain hooks 100% of the time.

Maybe we can do like a top-level React-Hookz Provider where we can give ourselves context regarding the users app being client-only or having any server-side code?

I think this would still mean conditional code though in a way you've previously pointed out has problems.

Alternatively, we could go full steam ahead and client-only users will just suffer the inconvenience.

[xobotyi]

This pr brings such context value to switch all hooks it one move

[kylemh]

lol yes it does 🤦🏼 ! sorry i was stuck thinking about that old PR and not this PR.

then yes, i love it in conjunction with the TS idea. seems like the best solution.

[ArttuOll]

It recently occurred to me that the new useSyncExternalStorage hook introduced in React 18 might allow us to remove some of the initializeWithValue parameters in our hooks.

It allows components to subscribe to external stores (in our case, browser APIs) and make React aware of their changes. The hook has a parameter called getServerSnapshot which allows us to define what the hook should return in a server environment and during hydration. To me it sounds like it solves the exact same problem that we are addressing with the initializeWithValue parameter.

Any thoughts?

[ArttuOll]

It recently occurred to me that the new useSyncExternalStorage hook introduced in React 18 might allow us to remove some of the initializeWithValue parameters in our hooks.

It allows components to subscribe to external stores (in our case, browser APIs) and make React aware of their changes. The hook has a parameter called getServerSnapshot which allows us to define what the hook should return in a server environment and during hydration. To me it sounds like it solves the exact same problem that we are addressing with the initializeWithValue parameter.

Any thoughts?

As an experiment, I attempted to port useWindowSize to use useSyncExternalStore under the hood. It greatly simplified the hooks implementation by removing all effects and the need for a separate initializeWithValue parameter. I see this as having huge potential for other hooks as well.

However, our toolchain would need some upgrading. The React teams has provided a package called use-sync-external-store which could enable users to use the hook in older React versions. When using this package, I couldn't get the server-side tests to work, which might be explained by the fact that react-hooks-testing-library is quite outdated since it has been merged to @testing-library/react and development continued there. However, starting from version 13, @testing-library/react has only supported React 18, which is problematic for us, since we attempt to support React versions >16.8.

Perhaps it would be time to reconsider which React versions we support?

[xobotyi]

Sadly had a few time lately, therefore havent tried that hook.

I'll try to cut some time this week to have a closer look.

[xobotyi]

@ArttuOll i just rememebered why we still not using react 18 and its hooks =)

the teting library do not support react 18, the solution they offer is switching to @testing-library/react that does not have any solution for SSR testing - they offer to write own testing library🙈