Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow an initial value to be returned by the data hooks #191

Merged
merged 4 commits into from
Dec 21, 2021

Conversation

dylanwatsonsoftware
Copy link
Contributor

@dylanwatsonsoftware dylanwatsonsoftware commented Dec 20, 2021

Resolves #190

As discussed, here is a PR to allow an initial value to be specified.

Questions that probably should be discussed:

  • Should it be 'initialValue' or 'defaultValue'?
  • Is it reasonable for it to be added as a field on the options param?

I also added a github action as I find them useful.. happy to remove if its not.
Its run doesn't seem to show up in this PR but you can find it here:
https://github.com/dylanwatsonsoftware/react-firebase-hooks/runs/4581537894?check_suite_focus=true

Merging this into the v3.0.x branch as this is the one that I'm forced to use due to a bug in firebase v9.

@dylanwatsonsoftware dylanwatsonsoftware changed the title All an initial value to be returned by the data hooks Allow an initial value to be returned by the data hooks Dec 20, 2021
Copy link
Contributor

@chrisbianca chrisbianca left a comment

Choose a reason for hiding this comment

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

@dylanwatsonsoftware This looks good to me, and I'm happy with initialValue rather than defaultValue. Would you be happy to add details of the option to the README?

@dylanwatsonsoftware
Copy link
Contributor Author

No dramas. I've had a go at adding something to the README. How's that?

@chrisbianca chrisbianca merged commit c520c3f into CSFrequency:v3.0.x Dec 21, 2021
@chrisbianca
Copy link
Contributor

Thanks @dylanwatsonsoftware, this looks great. I'll get a release out shortly

@chrisbianca
Copy link
Contributor

Release here: https://github.com/CSFrequency/react-firebase-hooks/releases/tag/v3.0.5

@dylanwatsonsoftware
Copy link
Contributor Author

Wow that was fast! Thanks so much!

@mauriceackel
Copy link
Contributor

Hi @chrisbianca I just wanted to use this feature in the current release (5.0.3) and was surprised to see that it is not part of this branch. Was there a reason for not having this as part of version 5? I have not seen any infos about this breaking change in the release notes.

I would love to see this feature in v5, so could we cherry pick those changes to the master branch?

Thanks, Maurice

@@ -123,7 +124,7 @@ const useCollectionDataInternal = <
transform
)
)
: undefined) as Data<T, IDField, RefField>[],
: options && options.initialValue) as Data<T, IDField, RefField>[],
Copy link
Contributor

Choose a reason for hiding this comment

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

If we promote this to v5, I would suggest we change this to options?.initialValue as an easier way of safe navigation

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.

3 participants