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

feat(HOCs): support running in react strict/concurrent mode #564

Closed
michaelspeed opened this issue Nov 13, 2018 · 16 comments
Closed

feat(HOCs): support running in react strict/concurrent mode #564

michaelspeed opened this issue Nov 13, 2018 · 16 comments

Comments

@michaelspeed
Copy link

Do you want to request a feature or report a bug?
bug

With react 16.7 alpha 0, enabling concurrent mode gives me this error -

`Warning: Unsafe lifecycle methods were found within a strict-mode tree:

componentWillMount: Please update the following components to use componentDidMount instead: FirebaseConnect(Connect(Core))

componentWillReceiveProps: Please update the following components to use static getDerivedStateFromProps instead: Connect(Core), FirebaseConnect(Connect(Core)), Provider

componentWillUpdate: Please update the following components to use componentDidUpdate instead: Connect(Core)`

@prescottprue
Copy link
Owner

@michaelspeed Thanks for reaching out with the warnings. There has been discussion of switching the HOCs to use componentDidMount for a while now (I believe the reasoning before was SSR). Initially I didn't do this since I wasn't sure if it was a breaking change or not, but through initial investigation, it doesn't seem to be.

Either way, I'll look into make a PR for the change.

@michaelspeed
Copy link
Author

but still the last warning is of {Connect} from react-redux package. i am not sure they are also planning to move to componentDidMount()

@prescottprue
Copy link
Owner

That is correct, though I am assuming that they will.

@prescottprue prescottprue mentioned this issue Nov 20, 2018
3 tasks
@prescottprue prescottprue changed the title Unable to Run in react strict mode / concurrent mode feat(HOCs): support running in react strict/concurrent mode Nov 20, 2018
@prescottprue
Copy link
Owner

Have started working on this in the v3.0.0-alpha branch, but there are some other changes I'm going to include before releasing.

prescottprue added a commit that referenced this issue Dec 15, 2018
* feat(HOCs): switch `firestoreConnect` and `firebaseConnect` to use `componentDidMount` over `componentWillMount` - #564 
* feat(core): New react context API working with HOCs (`firebaseConnect`, `firestoreConnect` `withFirebase`, and `withFirestore`) - #581
* feat(core): support `react-redux` v6 by removing usage of `context.store` (used to be added by `Provider` from `react-redux`)
* feat(core): Added `ReactReduxFirebaseContext` and `ReduxFirestoreContext` (with associated providers `ReactReduxFirebaseProvider` `ReduxFirestoreProvider` respectively)
* fix(core): shrink build sizes considerably by adding `babel-preset-minify` - #573
* feat(deps): update `hoist-non-react-statics` to `^3.1.0`
* feat(deps): upgrade to `babel^7`
* feat(deps): Update to `webpack^4` (along with `webpack-cli`)
* feat(deps): update react peer dependency to `^16.4` for new Context API (matches `react-redux`)
@prescottprue
Copy link
Owner

Released v3.0.0-alpha, which has these changes, to the next tag (it can be installed with npm i --save react-redux-firebase@next). Also published the v3.0.0 section of the docs which explains how to install and includes a migration guide.

@michaelspeed
Copy link
Author

thanks going to try this now

@michaelspeed
Copy link
Author

no exported member 'ReactReduxFirebaseProvider'
with type script? index.d.ts not updated?

@prescottprue
Copy link
Owner

@michaelspeed Yeah, the context's themselves were exported, so you can do ReactReduxFirebaseContext.Provider, but I also am planning on a releasing a change with the providers exported. Thanks for reaching out.

@cvanem
Copy link
Contributor

cvanem commented Jan 1, 2019

@prescottprue I tried v3.0.0-alpha.4 and am still getting the below error. Any idea how to fix this?

I am importing like this:

import { ReactReduxFirebaseProvider, ReactReduxFirebaseContext } from 'react-redux-firebase';

Error:

image

@prescottprue
Copy link
Owner

@cvanem I haven't yet released v3.0.0-alpha.4? Did you point to the branch itself?

Let me try publishing v3.0.0-alpha.4 which the typings change.

@cvanem
Copy link
Contributor

cvanem commented Jan 1, 2019

@prescottprue Yeah, I am working on the branch itself with yarn. I was able to get around the above error by doing the following:

var RRFirebaseProvider = compose()(ReactReduxFirebaseProvider) as React.ComponentType;

Then use as:

<RRFirebaseProvider {...rrfProps}>...</RRFirebaseProvider>

But I am still having problems getting it to work. I am debugging it now to try and figure out what is going on, but let me know if you have any ideas.

prescottprue added a commit that referenced this issue Jan 2, 2019
* fix(typings): add context providers to typings - #564
* fix(storage): support only firestore when calling upload and writing to db - @dirathea
* fix(core): add auth initialization to providers to match v2 store enhancer - #388
@prescottprue
Copy link
Owner

prescottprue commented Jan 2, 2019

@cvanem I made some changes and published v3.0.0-alpha.4 to npm. Can you give it a shot now and see if it works as expected?

@cvanem
Copy link
Contributor

cvanem commented Jan 2, 2019

@prescottprue Thanks, fixed the typescript error. Firebase is showing up in the redux store, but firebase.auth.isLoaded always equals false and I can't get authIsReady to fire. It may be something configured incorrectly on my end.

This is how my reducer is configured, which I believe is correct:

firebase.initializeApp(firebaseConfig);
firebase.auth();

...

function buildRootReducer(allReducers, history) {
    return combineReducers<ApplicationState>(Object.assign({}, allReducers, { router: connectRouter(history) }));
}

Also, are we still still suppose to use withFirebase to access the context or is there a better way now?

@cvanem
Copy link
Contributor

cvanem commented Jan 2, 2019

@prescottprue It looks like I needed to add the initializeAuth property. I removed firebase.auth() and used initializeAuth instead. This is what worked:

const rrfProps = {
  firebase,
  config: rrfConfig,
  dispatch: store.dispatch,
  initializeAuth: true,  
};

prescottprue pushed a commit that referenced this issue Jan 2, 2019
* fix(core): fix mispelled proptype on ReactReduxFirebaseProvider - #564
@prescottprue
Copy link
Owner

prescottprue commented Jan 2, 2019

@cvanem You shouldn't have to pass the initializeAuth prop yourself - seems like it was a misspelling in the default prop, so I am going to release a fix for that.

As for getting from context - Yes, withFirebase and firebaseConnect are both still good ways to get the wrapped Firebase instance. It is also available as a render prop if you use ReactReduxFirebaseContext.Consumer like so:

<ReactReduxFirebaseContext.Consumer>
  {firebase => (
     <SomeProp firebase={firebase} dispatch={firebase.dispatch} />
  )}
</ReactReduxFirebaseContext.Consumer>

Because of the option for above, I may end up exposing ReactReduxFirebaseContext.Consumer directly in the future

@prescottprue prescottprue mentioned this issue Jan 2, 2019
3 tasks
@cvanem
Copy link
Contributor

cvanem commented Jan 2, 2019

@prescottprue Ok, good to know. I tested v3.0.0 alpha.5 and everything is working as expected (without manually setting initializeAuth). Thanks.

prescottprue added a commit that referenced this issue Jan 2, 2019
* fix(context): fix misspelled `defaultProp` on `ReactReduxFirebaseProvider` - #564 
* feat(docs): add code sandbox example link to README
@prescottprue prescottprue mentioned this issue Oct 9, 2019
10 tasks
prescottprue added a commit that referenced this issue Oct 12, 2019
All changes from v3.0.0 pre-release versions including:
* [X] Hooks (`useFirebase`, `useFirebaseConnect`, `useFirestore`, `useFirestoreConnect`)
* [X] Rebuild on stable React Context API
* [X] Support react-redux v6 - [#581](#581)
* [X] Support for react strict mode - [#564](#564)
* [X] Improved bundle size support (should include way to audit size) - [#573](#573)
* [X] Support/Docs for stable react context API
* [X] Switch `firebaseConnect` and `firestoreConnect` to using `componentDidMount` in place of `componentWillMount` which is deprecated in newer react versions (as [described here](https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data))


More details available in [the v3.0.0 roadmap](https://github.com/prescottprue/react-redux-firebase/wiki/v3.0.0-Roadmap)
mirdavion pushed a commit to mirdavion/react-redux-firebase that referenced this issue Mar 18, 2021
All changes from v3.0.0 pre-release versions including:
* [X] Hooks (`useFirebase`, `useFirebaseConnect`, `useFirestore`, `useFirestoreConnect`)
* [X] Rebuild on stable React Context API
* [X] Support react-redux v6 - [#581](prescottprue/react-redux-firebase#581)
* [X] Support for react strict mode - [#564](prescottprue/react-redux-firebase#564)
* [X] Improved bundle size support (should include way to audit size) - [#573](prescottprue/react-redux-firebase#573)
* [X] Support/Docs for stable react context API
* [X] Switch `firebaseConnect` and `firestoreConnect` to using `componentDidMount` in place of `componentWillMount` which is deprecated in newer react versions (as [described here](https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data))


More details available in [the v3.0.0 roadmap](https://github.com/prescottprue/react-redux-firebase/wiki/v3.0.0-Roadmap)
prodev90 added a commit to prodev90/react-redux-firebase-sample that referenced this issue Jan 4, 2023
All changes from v3.0.0 pre-release versions including:
* [X] Hooks (`useFirebase`, `useFirebaseConnect`, `useFirestore`, `useFirestoreConnect`)
* [X] Rebuild on stable React Context API
* [X] Support react-redux v6 - [#581](prescottprue/react-redux-firebase#581)
* [X] Support for react strict mode - [#564](prescottprue/react-redux-firebase#564)
* [X] Improved bundle size support (should include way to audit size) - [#573](prescottprue/react-redux-firebase#573)
* [X] Support/Docs for stable react context API
* [X] Switch `firebaseConnect` and `firestoreConnect` to using `componentDidMount` in place of `componentWillMount` which is deprecated in newer react versions (as [described here](https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data))


More details available in [the v3.0.0 roadmap](https://github.com/prescottprue/react-redux-firebase/wiki/v3.0.0-Roadmap)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants