search

devcshort / stack-inbox

Chrome Extension for Stack Overflow Inbox and Notifications
MIT License
4 stars 6 forks source link

App throwing error on first time clone #13

Closed bhavin-a closed 4 years ago

bhavin-a commented 4 years ago

Hi

I cloned this repo and started the server. At the initial loads, I see an error. Please refer the screenshot: image

I am suspecting this due to an unhandled check for the local storage key of "notification". Can you please confirm if this is reproducible? Or am I missing something?

If this is a genuine case, would like to contribute a small check to the code. Thanks!

bhavin-a commented 4 years ago

@devcshort Can I work on this bug? Thanks.

devcshort commented 4 years ago

@Bhavin789 I just realized that there is an issue with my instructions for the README. Currently you can't run the extension with yarn start, you have to do a build and have the built package be installed in the Chrome browser. So this bug sounds like it's probably expected. This app relies on the Chrome background tasks to be able to fetch notifications automatically in the background.

It would probably still be good to have the additional check to be sure that the notifications array exists, so feel free to make the change and submit a PR. :)

bhavin-a commented 4 years ago

Oh understood @devcshort , thanks for clarity here!

So I was thinking to keep the array as empty (if we don't find "notification" key in the local storage) and show the "login message" which is by default - just to be sure that our React page doesn't break. Please suggest some other message if you feel so. Would like to then raise a PR.

Thanks!

devcshort commented 4 years ago

@Bhavin789 Yes that sounds like a great idea. The login messages should be fine here.

bhavin-a commented 4 years ago

Awesome, let me work on this quick fix then! Thanks :)