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

question(auth): sign in anonymously on application starup #137

Closed
radicalnerds opened this Issue May 18, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@radicalnerds

radicalnerds commented May 18, 2017

I am trying to add an onAuthStateChanged listener to the firebase object when using reactReduxFirebase. The docs here describe that this parameters should set that value.
http://react-redux-firebase.com/docs/api/compose.html

My assumption is like in the tutorial here
https://www.youtube.com/watch?v=ApG8L2RKrSI

onAuthStateChanged will get called on appStartup w/ the current user or an undefined user (which in my code will trigger an anonymous login). My goal is to make my app by default be an anonymously logged in user. Is something wrong with my approach here? Is this feature supposed to work as I have described?

`javascript
import { AsyncStorage } from 'react-native';
import { reactReduxFirebase } from 'react-redux-firebase';

// Firebase config
const firebaseConfig = {
apiKey: '',
authDomain: '',
databaseURL: '',
projectId: '',
storageBucket: '',
messagingSenderId: '',
};

// react-redux-firebase options
const config = {
userProfile: 'users',
enableLogging: false,
ReactNative: { AsyncStorage },
onAuthStateChanged: (user, firebase) => {
console.log('User INFORMATION BRO', user);
if (user) {
// User is signed in.
} else {
firebase.auth.signInAnonymously();
}
},
};

const FirebaseStore = reactReduxFirebase(firebaseConfig, config);
export default FirebaseStore;

// Eventually store is created like this
const createStoreWithFirebase = compose(FirebaseStore)(createStore);
const store = createStoreWithFirebase(reducers);
`

@prescottprue

This comment has been minimized.

Show comment
Hide comment
@prescottprue

prescottprue May 19, 2017

Owner

Based on your code, this is expected behavior (the onAuthStateChanged callback in react-redux-firebase is not called if user is undefined, instead the "LOGOUT" action is dispatched). Your auth state has not actually changed at all (it is always just logged out).

This means that the tutorial you have posted will not work.

It seems like you are attempting to sign in anonymously on application start. If that is the case you will want to be checking to see if auth exists on that boot not when auth changes.

You would most likely want to place the sign in anonymously code into a home page component. That page can check auth, redirecting somewhere if it exists, and bringing you to a login page if not. That login page can handle the signing in anonymously.

All of that said, just to make things more clear, this may be added as a feature in the future, but will be breaking for some.

Owner

prescottprue commented May 19, 2017

Based on your code, this is expected behavior (the onAuthStateChanged callback in react-redux-firebase is not called if user is undefined, instead the "LOGOUT" action is dispatched). Your auth state has not actually changed at all (it is always just logged out).

This means that the tutorial you have posted will not work.

It seems like you are attempting to sign in anonymously on application start. If that is the case you will want to be checking to see if auth exists on that boot not when auth changes.

You would most likely want to place the sign in anonymously code into a home page component. That page can check auth, redirecting somewhere if it exists, and bringing you to a login page if not. That login page can handle the signing in anonymously.

All of that said, just to make things more clear, this may be added as a feature in the future, but will be breaking for some.

@prescottprue prescottprue changed the title from Bug: Setting onAuthStateChanged results in no call to it. to question(auth): sign in anonymously on application starup May 19, 2017

@radicalnerds

This comment has been minimized.

Show comment
Hide comment
@radicalnerds

radicalnerds May 19, 2017

Great thanks for the quick reply! Just to be 100% clear this is a deviation of how this method is normally used for the 'firebase' npm package which this is based on correct?

Appreciate the suggested implementation!

radicalnerds commented May 19, 2017

Great thanks for the quick reply! Just to be 100% clear this is a deviation of how this method is normally used for the 'firebase' npm package which this is based on correct?

Appreciate the suggested implementation!

@prescottprue

This comment has been minimized.

Show comment
Hide comment
@prescottprue

prescottprue May 19, 2017

Owner

@radicalnerds Yes, it is a deviation.

That said, when that decision was made, this use case was not understood. One of the goals of this library is to keep compatibility with Firebase across the board, so this is for sure a good idea for a change.

Thanks for pointing it out. This issue will be updated when/if this is changed.

Owner

prescottprue commented May 19, 2017

@radicalnerds Yes, it is a deviation.

That said, when that decision was made, this use case was not understood. One of the goals of this library is to keep compatibility with Firebase across the board, so this is for sure a good idea for a change.

Thanks for pointing it out. This issue will be updated when/if this is changed.

@prescottprue prescottprue referenced this issue May 23, 2017

Merged

Version v1.5.0-alpha #136

3 of 3 tasks complete

prescottprue added a commit that referenced this issue May 23, 2017

Version v1.5.0-alpha (#136)
### Description
* Adds `updateProfile`, `updateAuth`, and `updateEmail` methods that dispatch associated start/success/failure actions
* Adds multiple features to populate - #132
* Uses `prop-types` instead of importing from react - #122 
* `distpatchOnUnsetListener` fixed to be `dispatchOnUnsetListener` (depreciation warning added along with a test confirm it is displayed)
* Do not include `dbPath` in response from `deleteFile` method if it is undefined (test added to check this case)
* `.eslintrc` file now using yaml format instead of JSON format (easier to read)
* Unnessesary global eslint comments removed from tests (no longer needed due to globals being moved to `.eslintrc`)
* `enableEmptyAuthChanges` config option added - #137

### Check List

- [X] All tests passing
- [X] Docs updated with any changes or examples
- [X] Added tests to ensure feature(s) work properly

### Relevant Issues
* [#122](https://github.com/prescottprue/react-redux-firebase/issues/#122)
* [#132](https://github.com/prescottprue/react-redux-firebase/issues/#132)
* [#137](https://github.com/prescottprue/react-redux-firebase/issues/#137)
@prescottprue

This comment has been minimized.

Show comment
Hide comment
@prescottprue

prescottprue May 23, 2017

Owner

version v1.5.0-alpha has been released and includes the enableEmptyAuthChanges config option. If this is set to true, onAuthStateChanged will be fired with undefined on initialization as you were originally expecting.

As stated, this may become the standard functionality, but it has remained disabled by default for now to keep new onAuthStateChanged behavior from being a breaking change.

Owner

prescottprue commented May 23, 2017

version v1.5.0-alpha has been released and includes the enableEmptyAuthChanges config option. If this is set to true, onAuthStateChanged will be fired with undefined on initialization as you were originally expecting.

As stated, this may become the standard functionality, but it has remained disabled by default for now to keep new onAuthStateChanged behavior from being a breaking change.

@prescottprue prescottprue referenced this issue Sep 7, 2017

Merged

v1.5.0 #270

prescottprue added a commit that referenced this issue Sep 18, 2017

v1.5.0 (#270)
🍾 🥂 The long awaited v1.5.0 release 🥂 🍾 

### Breaking Changes
* `browser` field has been removed from `package.json` which means commonJS build is used instead of `UMD` build from Webpack (much smaller)
* [firebaseConnect] Uses `prop-types` instead of importing from React - #122 

### Core
* [firebaseConnect] Allow usage of different stores in `firebaseConnect` - #148, #29
* [firebaseConnect] statics are now hoisted thanks to `hoist-non-react-statics`
* [firebaseConnect] exposed `createFirebaseConnect` for creating `firebaseConnect` HOCs connected to different stores
* [auth] `login` method supports `credential` parameter (with matching docs and tests updates)
* [auth] deprecation warning added for `token` and `provider` combo in `login` method
* [auth] Adds `updateProfile`, `updateAuth`, and `updateEmail` methods that dispatch associated start/success/failure actions
* [helpers] `deleteFile` method response no longer includes `dbPath` if it is undefined (test added to check this case)
* [helpers] No more race condition with `uniqueSet` - #207
* [typescript] Typescript typings - #142, #214 
* [query] `watchEvent` accepts options object as third argument
* [populate] Lodash path syntax support - #132
* [populate] accept a function for populate for item based populate config - #132
* [populate] Profile population (`profileParamsToPopulate`) supports single item and list population - #203
* [storage] `uploadFile` and `uploadFiles` no longer track progress by default (`storageRef.put` called directly)
* [config] Usage of `profileFactory` is wrapped in try/catch to handle and reject errors within provided factory function
* [config] `distpatchOnUnsetListener` spelling fixed to be `dispatchOnUnsetListener` (depreciation warning added along with a test confirm it is displayed)
* [config] `enableEmptyAuthChanges` config option added - #137


### Build
* Firebase is now an external in Webpack config (shrinks UMD build size)
* `.npmignore` expanded to include more files that do not needed to be included in published version (including `.babelrc` which can cause build errors)
* `.babelrc` settings are now environment specific (decorators only used in testing)
* `babel-preset-stage-1` removed (unnecessary)
* `.eslintrc` added to tests folder which contains globals that are only necessary for testing (simplifies top level eslint config)
* `.eslintrc` file now using yaml format instead of JSON format (easier to read)
* `async/await` added to unit tests
* Webpack 2
* [`firebase-server`](https://github.com/urish/firebase-server) used in tests instead of connecting to a real Firebase instance

### Docs
* Query docs updated with `storeAs` and `keyProp` query options
* `redux-persist` recipe added

prescottprue added a commit that referenced this issue Sep 18, 2017

v1.5.0 (#270)
🍾 🥂 The long awaited v1.5.0 release 🥂 🍾

* `browser` field has been removed from `package.json` which means commonJS build is used instead of `UMD` build from Webpack (much smaller)
* [firebaseConnect] Uses `prop-types` instead of importing from React - #122

* [firebaseConnect] Allow usage of different stores in `firebaseConnect` - #148, #29
* [firebaseConnect] statics are now hoisted thanks to `hoist-non-react-statics`
* [firebaseConnect] exposed `createFirebaseConnect` for creating `firebaseConnect` HOCs connected to different stores
* [auth] `login` method supports `credential` parameter (with matching docs and tests updates)
* [auth] deprecation warning added for `token` and `provider` combo in `login` method
* [auth] Adds `updateProfile`, `updateAuth`, and `updateEmail` methods that dispatch associated start/success/failure actions
* [helpers] `deleteFile` method response no longer includes `dbPath` if it is undefined (test added to check this case)
* [helpers] No more race condition with `uniqueSet` - #207
* [typescript] Typescript typings - #142, #214
* [query] `watchEvent` accepts options object as third argument
* [populate] Lodash path syntax support - #132
* [populate] accept a function for populate for item based populate config - #132
* [populate] Profile population (`profileParamsToPopulate`) supports single item and list population - #203
* [storage] `uploadFile` and `uploadFiles` no longer track progress by default (`storageRef.put` called directly)
* [config] Usage of `profileFactory` is wrapped in try/catch to handle and reject errors within provided factory function
* [config] `distpatchOnUnsetListener` spelling fixed to be `dispatchOnUnsetListener` (depreciation warning added along with a test confirm it is displayed)
* [config] `enableEmptyAuthChanges` config option added - #137

* Firebase is now an external in Webpack config (shrinks UMD build size)
* `.npmignore` expanded to include more files that do not needed to be included in published version (including `.babelrc` which can cause build errors)
* `.babelrc` settings are now environment specific (decorators only used in testing)
* `babel-preset-stage-1` removed (unnecessary)
* `.eslintrc` added to tests folder which contains globals that are only necessary for testing (simplifies top level eslint config)
* `.eslintrc` file now using yaml format instead of JSON format (easier to read)
* `async/await` added to unit tests
* Webpack 2
* [`firebase-server`](https://github.com/urish/firebase-server) used in tests instead of connecting to a real Firebase instance

* Query docs updated with `storeAs` and `keyProp` query options
* `redux-persist` recipe added

prescottprue added a commit that referenced this issue Sep 18, 2017

v1.5.0 (#270)
🍾 🥂 The long awaited v1.5.0 release 🥂 🍾

* `browser` field has been removed from `package.json` which means commonJS build is used instead of `UMD` build from Webpack (much smaller)
* [firebaseConnect] Uses `prop-types` instead of importing from React - #122

* [firebaseConnect] Allow usage of different stores in `firebaseConnect` - #148, #29
* [firebaseConnect] statics are now hoisted thanks to `hoist-non-react-statics`
* [firebaseConnect] exposed `createFirebaseConnect` for creating `firebaseConnect` HOCs connected to different stores
* [auth] `login` method supports `credential` parameter (with matching docs and tests updates)
* [auth] deprecation warning added for `token` and `provider` combo in `login` method
* [auth] Adds `updateProfile`, `updateAuth`, and `updateEmail` methods that dispatch associated start/success/failure actions
* [helpers] `deleteFile` method response no longer includes `dbPath` if it is undefined (test added to check this case)
* [helpers] No more race condition with `uniqueSet` - #207
* [typescript] Typescript typings - #142, #214
* [query] `watchEvent` accepts options object as third argument
* [populate] Lodash path syntax support - #132
* [populate] accept a function for populate for item based populate config - #132
* [populate] Profile population (`profileParamsToPopulate`) supports single item and list population - #203
* [storage] `uploadFile` and `uploadFiles` no longer track progress by default (`storageRef.put` called directly)
* [config] Usage of `profileFactory` is wrapped in try/catch to handle and reject errors within provided factory function
* [config] `distpatchOnUnsetListener` spelling fixed to be `dispatchOnUnsetListener` (depreciation warning added along with a test confirm it is displayed)
* [config] `enableEmptyAuthChanges` config option added - #137

* Firebase is now an external in Webpack config (shrinks UMD build size)
* `.npmignore` expanded to include more files that do not needed to be included in published version (including `.babelrc` which can cause build errors)
* `.babelrc` settings are now environment specific (decorators only used in testing)
* `babel-preset-stage-1` removed (unnecessary)
* `.eslintrc` added to tests folder which contains globals that are only necessary for testing (simplifies top level eslint config)
* `.eslintrc` file now using yaml format instead of JSON format (easier to read)
* `async/await` added to unit tests
* Webpack 2
* [`firebase-server`](https://github.com/urish/firebase-server) used in tests instead of connecting to a real Firebase instance

* Query docs updated with `storeAs` and `keyProp` query options
* `redux-persist` recipe added

prescottprue added a commit that referenced this issue Sep 18, 2017

v1.5.0 (#270)
🍾 🥂 The long awaited v1.5.0 release 🥂 🍾

* `browser` field has been removed from `package.json` which means commonJS build is used instead of `UMD` build from Webpack (much smaller)
* [firebaseConnect] Uses `prop-types` instead of importing from React - #122

* [firebaseConnect] Allow usage of different stores in `firebaseConnect` - #148, #29
* [firebaseConnect] statics are now hoisted thanks to `hoist-non-react-statics`
* [firebaseConnect] exposed `createFirebaseConnect` for creating `firebaseConnect` HOCs connected to different stores
* [auth] `login` method supports `credential` parameter (with matching docs and tests updates)
* [auth] deprecation warning added for `token` and `provider` combo in `login` method
* [auth] Adds `updateProfile`, `updateAuth`, and `updateEmail` methods that dispatch associated start/success/failure actions
* [helpers] `deleteFile` method response no longer includes `dbPath` if it is undefined (test added to check this case)
* [helpers] No more race condition with `uniqueSet` - #207
* [typescript] Typescript typings - #142, #214
* [query] `watchEvent` accepts options object as third argument
* [populate] Lodash path syntax support - #132
* [populate] accept a function for populate for item based populate config - #132
* [populate] Profile population (`profileParamsToPopulate`) supports single item and list population - #203
* [storage] `uploadFile` and `uploadFiles` no longer track progress by default (`storageRef.put` called directly)
* [config] Usage of `profileFactory` is wrapped in try/catch to handle and reject errors within provided factory function
* [config] `distpatchOnUnsetListener` spelling fixed to be `dispatchOnUnsetListener` (depreciation warning added along with a test confirm it is displayed)
* [config] `enableEmptyAuthChanges` config option added - #137

* Firebase is now an external in Webpack config (shrinks UMD build size)
* `.npmignore` expanded to include more files that do not needed to be included in published version (including `.babelrc` which can cause build errors)
* `.babelrc` settings are now environment specific (decorators only used in testing)
* `babel-preset-stage-1` removed (unnecessary)
* `.eslintrc` added to tests folder which contains globals that are only necessary for testing (simplifies top level eslint config)
* `.eslintrc` file now using yaml format instead of JSON format (easier to read)
* `async/await` added to unit tests
* Webpack 2
* [`firebase-server`](https://github.com/urish/firebase-server) used in tests instead of connecting to a real Firebase instance

* Query docs updated with `storeAs` and `keyProp` query options
* `redux-persist` recipe added

prescottprue added a commit that referenced this issue Sep 18, 2017

v1.5.0 (#270)
🍾 🥂 The long awaited v1.5.0 release 🥂 🍾

* `browser` field has been removed from `package.json` which means commonJS build is used instead of `UMD` build from Webpack (much smaller)
* [firebaseConnect] Uses `prop-types` instead of importing from React - #122

* [firebaseConnect] Allow usage of different stores in `firebaseConnect` - #148, #29
* [firebaseConnect] statics are now hoisted thanks to `hoist-non-react-statics`
* [firebaseConnect] exposed `createFirebaseConnect` for creating `firebaseConnect` HOCs connected to different stores
* [auth] `login` method supports `credential` parameter (with matching docs and tests updates)
* [auth] deprecation warning added for `token` and `provider` combo in `login` method
* [auth] Adds `updateProfile`, `updateAuth`, and `updateEmail` methods that dispatch associated start/success/failure actions
* [helpers] `deleteFile` method response no longer includes `dbPath` if it is undefined (test added to check this case)
* [helpers] No more race condition with `uniqueSet` - #207
* [typescript] Typescript typings - #142, #214
* [query] `watchEvent` accepts options object as third argument
* [populate] Lodash path syntax support - #132
* [populate] accept a function for populate for item based populate config - #132
* [populate] Profile population (`profileParamsToPopulate`) supports single item and list population - #203
* [storage] `uploadFile` and `uploadFiles` no longer track progress by default (`storageRef.put` called directly)
* [config] Usage of `profileFactory` is wrapped in try/catch to handle and reject errors within provided factory function
* [config] `distpatchOnUnsetListener` spelling fixed to be `dispatchOnUnsetListener` (depreciation warning added along with a test confirm it is displayed)
* [config] `enableEmptyAuthChanges` config option added - #137

* Firebase is now an external in Webpack config (shrinks UMD build size)
* `.npmignore` expanded to include more files that do not needed to be included in published version (including `.babelrc` which can cause build errors)
* `.babelrc` settings are now environment specific (decorators only used in testing)
* `babel-preset-stage-1` removed (unnecessary)
* `.eslintrc` added to tests folder which contains globals that are only necessary for testing (simplifies top level eslint config)
* `.eslintrc` file now using yaml format instead of JSON format (easier to read)
* `async/await` added to unit tests
* Webpack 2
* [`firebase-server`](https://github.com/urish/firebase-server) used in tests instead of connecting to a real Firebase instance

* Query docs updated with `storeAs` and `keyProp` query options
* `redux-persist` recipe added
@prescottprue

This comment has been minimized.

Show comment
Hide comment
@prescottprue

prescottprue Oct 26, 2017

Owner

Just as a heads up, this is being removed from v2.0.0. It is replaced by preserveOnEmptyAuthChange, checkout the migration guide for examples.

Owner

prescottprue commented Oct 26, 2017

Just as a heads up, this is being removed from v2.0.0. It is replaced by preserveOnEmptyAuthChange, checkout the migration guide for examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment