Fix initializeWithValue default not respected, fix docs and tests

[ArttuOll]

What is the current behavior, and the steps to reproduce the issue?

initializeWithValue default value is not respected. It is always read from user-given options, meaning that it is undefined (effectively false), if the user did not explicitly set it.

Reproduction steps are given in #1002 . It looks like initializeWithValue is false by default (which is what the bug report is about), but actually it's just that the default value of true is not respected.

Additionally, I noticed that the default value for initializeWithValue is set to true, which partly contradicts the documentation (in the top part of the docs of useSessionStorageValue and useLocalStorageValue initializeWithValue is said to be false by default, but in the description of the options argument it is said to be true) and the tests. After fixing useStorageValue to respect the default value of initializeWithValue it became necessary to set it to false, otherwise SSR tests would fail, as they should.

What is the expected behavior?

The initializeWithValue default should be respected and it should be false by default.

How does this PR fix the problem?

• Make sure the default value of initializeWithValue is used if it is not provided by the user.
• Set initializeWithValue to false by default.
  • Note, that this is not a breaking change, because previously it was already effectively false.

Checklist

• Have you read contribution guideline?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Is there an existing issue for this PR?
  • useLocalStorageValue yields undefined on firstRender if initializeWithValue not set #1002
• Have the files been linted and formatted?
• Have the docs been updated to match the changes in the PR?
• Have the tests been updated to match the changes in the PR?
• Have you run the tests locally to confirm they pass?

[codecov]

Codecov Report

Merging #1003 (81711e2) into master (7bcc385) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1003   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files          61       61           
  Lines        1039     1039           
  Branches      165      165           
=======================================
  Hits         1035     1035           
  Misses          2        2           
  Partials        2        2           

Impacted Files	Coverage Δ
src/useStorageValue/useStorageValue.ts	100.00% <100.00%> (ø)

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

[Evalon]

Hi!
Thank you for quick response and fix. I'm not sure about setting initializeWithValue to be false by default. In mind of developers useLocalStorage is the abstraction that works similar to useState but saves value between user sessions in local storage. So if it yields undefined first and then yields actual value it breaks this abstraction.
Can we support this abstraction by default?

[ArttuOll]

Hi! Thank you for quick response and fix. I'm not sure about setting initializeWithValue to be false by default. In mind of developers useLocalStorage is the abstraction that works similar to useState but saves value between user sessions in local storage. So if it yields undefined first and then yields actual value it breaks this abstraction. Can we support this abstraction by default?

I understand your concern. However, supporting SSR is one of the key goals of this library. Settings initializeWithValue to false by default means SSR is supported out of the box. I'm interested to hear what my fellow maintainers have to say about this.

[xobotyi]

That is the change from previous implementation, previously SSR users had to set this option to false, wheras this time it is set to false by default. I dont have complete decision for it - maybe we have to change it back to true

[xobotyi]

So basically #1003 is about changed default value and docs not being fully actualized.

[xobotyi]

@ArttuOll I'd like you to make default value to be true. Also docs has to be actualized.

[ArttuOll]

I see. I'll edit this that way then. That'll require some larger changes to the tests.

[xobotyi]

yeah, sorry🙏 been distracted during design of reworked hook 🙈

[Evalon]

Yeah, reflecting on it a little bit more.
I also don't have a good decision for this. Defaults in context (like ssr=true)? Static initialization? Different hooks for CSR?
I fully understand your concerns.
If initializeWithValue stays false then I need to make a wrapper around useLocalStorageValue and if it changes to true SSR-users need to make a wrapper like this. Because, it's inconvenient to remember to specify the same options everywhere in the case of CSR or SSR.

[xobotyi]

or you can just know that you're using ssr ant set proper field. by now - none been complaining, since it is only hook (at least that i know) that supports ssr\csr duality🙃

[ArttuOll]

Btw, this becomes a breaking change now since we are setting initializeWithValue to true while it was previously effectively false.

[xobotyi]

That was a 'bug' 🙃
Since default value was true - I just made a mistake on read source.

[xobotyi]

🎉 This PR is included in version 17.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀