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

ReactNative + react-native-firebase + Figwheel issue. #638

Closed
boogie666 opened this issue Jan 12, 2018 · 1 comment · Fixed by #639
Closed

ReactNative + react-native-firebase + Figwheel issue. #638

boogie666 opened this issue Jan 12, 2018 · 1 comment · Fixed by #639

Comments

@boogie666
Copy link
Contributor

Hello :)

I've found an issue in this very wired situation on ReactNative using cljs and Figwheel and react-native-firebase.

support/src/figwheel/client/utils.cljs needs localStorage to get some configs and does 'feature' detection to see if it has localStorage

see here https://github.com/bhauman/lein-figwheel/blob/master/support/src/figwheel/client/utils.cljs#L138

While under normal circumstances this would be sufficient, it just so happens that react-native-firebase
For some stupid reason does this
https://github.com/invertase/react-native-firebase/blob/master/lib/utils/log.js#L11

Basically if localStorage isn't there it assigns an empty object... -.- ....

This is clearly a bad idea on them but... it's probably not the only lib that does this.
In an ideal world what figwheel does is perfect, sadly the world is not ideal

Imo the figwheel client should check for localStorage features a bit better.

I've created a fix for this by adding better feature detection for localStorage (basically checking if the object exists and has the correct method)

See here https://github.com/boogie666/lein-figwheel/blob/master/support/src/figwheel/client/utils.cljs#L114

I'll submit a pull request.

Cheers

@kh0r
Copy link

kh0r commented Mar 6, 2018

Unfortunately this fix has false assumptions. js/localStorage throws Error: Uncaught ReferenceError: localStorage is not defined before the new feature? function can even start its checks. The correct way would be to use js/window.localStorage here. This returns undefined and the checks perform as intended.

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 a pull request may close this issue.

2 participants