search

bhauman / lein-figwheel

Figwheel builds your ClojureScript code and hot loads it into the browser as you are coding!
Eclipse Public License 1.0
2.88k stars 210 forks source link

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

Closed boogie666 closed 6 years ago

boogie666 commented 6 years ago

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 commented 6 years ago

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.