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

Add firestore reducer for typescript #296

Closed

Conversation

bawahakim
Copy link
Contributor

Description

I couldn't properly get firestore to work with Typescript, so I went ahead and tried implementing it.

Check List

If not relevant to pull request, check off as complete

  • All tests passing
  • Docs updated with any changes or examples if applicable
  • Added tests to ensure new feature(s) work properly

Relevant Issues

@codecov
Copy link

codecov bot commented Jul 10, 2020

Codecov Report

Merging #296 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #296   +/-   ##
=======================================
  Coverage   90.09%   90.09%           
=======================================
  Files          19       19           
  Lines         626      626           
=======================================
  Hits          564      564           
  Misses         62       62           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7a5613...f8857f6. Read the comment docs.

@msutkowski
Copy link
Contributor

@feelthepain444 You might want to check this out this PR I closed: prescottprue/react-redux-firebase#964. I'd be happy to collaborate with you on this - I meant to open this against the typescript branch here, but I have no clue as to what's going on with this repo so I've just been keeping my types locally.

Regardless, you'd want to return a Reducer type from redux on firestoreReducer, otherwise you'd still have to cast in combineReducers. And FirestoreReducer.Reducer should probably be more like FirestoreReducer.State.

@alexandermckay
Copy link

@msutkowski

@bawahakim is no longer using redux-firestore or react-redux-firestore link.

I am guessing here but I assume he understandably won't want to spend anymore time on this. I will fork his repo over the weekend and add the changes you've suggested.

@devth
Copy link

devth commented Feb 11, 2021

Did anything ever progress with this?

prescottprue added a commit that referenced this pull request Feb 15, 2021
Types based on work shared by @msutkowski, and @bawahakim

Co-Authored-By: Matt Sutkowski <784953+msutkowski@users.noreply.github.com>
Co-Authored-By: Hakim Bawa <43802542+bawahakim@users.noreply.github.com>
@prescottprue prescottprue mentioned this pull request Feb 15, 2021
3 tasks
prescottprue added a commit that referenced this pull request Feb 15, 2021
* fix(getFirestore): correctly load extended firestore instance in `getFirestore` (#318) - @aarajh 
* fix(orderedReducer): reorder items when `newIndex` is zero (#317) - @matthew-h-cromer
* feat(types): add types for reducer (#296, [react-redux-firebase 964](prescottprue/react-redux-firebase#964)) - @msutkowski,@bawahakim
* fix(examples): update complete example patterns + major versions
* build(deps): bump immer from 5.0.0 to 8.0.1 (#320)
* build(deps): bump ini from 1.3.5 to 1.3.8 (#316)
* build(deps): bump ini from 1.3.5 to 1.3.8 in /examples/basic (#315)

Co-Authored-By: Hakim Bawa <43802542+bawahakim@users.noreply.github.com>
Co-authored-by: Aaraj Habib <aarajh@gmail.com>
Co-authored-by: matthew-h-cromer <35811043+matthew-h-cromer@users.noreply.github.com>
Co-authored-by: Matt Sutkowski <784953+msutkowski@users.noreply.github.com>
@prescottprue
Copy link
Owner

v0.15.0 release has the functionality based on code here and @msutkowski mentioned PR - because of that both of you were listed as co-authors of the commit while I was manually combining it

This is a first swing at the types, but it seemed to be working in a simple example. Please reach out if things aren't working as expected!

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.

5 participants