Closed mlampedx closed 4 years ago
Overall this implementation looks great. Thank you for working on initial value support and the corresponding tests, @mlampedx!
I would love for us to check how far this addresses @gja's original concerns in #18 and sanity check how we might want to factor in https://github.com/GoogleChromeLabs/react-adaptive-hooks/issues/18#issuecomment-554485731 or https://github.com/GoogleChromeLabs/react-adaptive-hooks/issues/18#issuecomment-553965505. I think it's completely fair if we address the rest of attribute mismatch in a separate PR, but a brief discussion before landing this is probably healthy :)
Thank you, @addyosmani ! I have addressed your comments and followed up on the original issue thread.
Thank you, @mlampedx. I think there's enough value in the initial value arguments implementation here for us to land it and then make a decision on the right design for the second half of the problem over in #18. Approving and landing shortly 🚀
Purpose:
I am proposing that we add an optional initial value argument to the
useNetworkStatus
,useSaveData
, anduseMemoryStatus
hooks to enable the developer to provide an alternative source for this valuable data in the event that the JavaScript runtime environment/the user's browser lacks the required APIs.This opens up the opportunity for developers to utilize relevant server-side client hints that are available. For example, in instances where the server-side client hint value is reliable and consistent with the navigator value, populating the useNetworkStatus hook on the server can enable adaptive loading on SSR, reduce re-renders, and ensure HTML is consistent between SSR and client-side hydration.
Completed:
useNetworkStatus
,useSaveData
;useMemoryStatus
unsupported
property to all states returned by hooks for consistencyPlease let me know if you would like me to amend, revert, or clarify any of the changes that I am proposing 😄