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

Feature/initial value #270

Merged
merged 2 commits into from
Oct 27, 2022
Merged

Feature/initial value #270

merged 2 commits into from
Oct 27, 2022

Conversation

mauriceackel
Copy link
Contributor

This is a PR related to #190 and #191.

It adds the possibility to define an initial value for the

  • useCollectionDataOnce
  • useCollectionData
  • useDocumentDataOnce
  • useDocumentData
    hooks.

It is heavily based on PR #191 but as this PR was merged to the 3.0.x branch, it is not part of the current version (i.e. 5.0.x).

I've slightly changed the code compared to #191 and I also fixed some typing issues.

@chrisbianca, please let me know if I could somehow help in making this part of version 5 swiftly.

Thanks, Maurice

@mauriceackel
Copy link
Contributor Author

@dylanwatsonsoftware, I would love to to get your opinions on this, given that you created the original feature request an PR.

@dylanwatsonsoftware
Copy link
Contributor

dylanwatsonsoftware commented Oct 11, 2022

LGTM! ✅

The only thing that I can see missing compared to my original PR is the GitHub action. Though that is obviously unrelated to the actual feature, it does help prove that the code isn't totally broken 🤪

Thoughts @chrisbianca ?

@mauriceackel
Copy link
Contributor Author

mauriceackel commented Oct 26, 2022

Hey @chrisbianca, did you have time to look into this by any chance? 🙂

@chrisbianca chrisbianca merged commit f41dbc8 into CSFrequency:master Oct 27, 2022
@chrisbianca
Copy link
Contributor

@mauriceackel This looks great, thanks for the submission and pulling through the functionality that was clearly missed when I did the last refactor. I'll get a release out in the coming days.

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