Error on initial load about redux-localstorage-simple HOT 14 CLOSED

@kilkelly I've created a new PR #13 for a config flag to disable warnings, and changed console.error to warn for missing namespaces.

from redux-localstorage-simple.

@kilkelly I've upgraded the version for our project and it seems to work just fine. Thanks again!

from redux-localstorage-simple.

@kilkelly All types of logs are being displayed in my CI testing (my prod should not include any). I could make adjustments to suppress console.info and/or console.log though, but I guess I don't have an understanding why this instance is being logged at all. Perhaps just a comment in the source would be sufficient seeing as everything works fine when this path is taken?

from redux-localstorage-simple.

@kilkelly @jesperlehtinen I think either would be a valid solution. Honestly having both implemented would be the most ideal situation in my opinion - maybe change it to use console.warn instead and have an option to suppress all warnings as a config like disableWarnings: true with false being the default.

from redux-localstorage-simple.

Perfect! Thanks guys! Closing the issue.

from redux-localstorage-simple.

I am using console.error to.display this message. However I am now thinking to use console.warn or console.info as it's ambiguous whether this situation warrants an error message. Would either of these alternatives trigger your CI tests? I should also rephrase this message to better explain why this situation happens.

from redux-localstorage-simple.

I don't understand why this is considered an error at all. When loading from localStorage for the very first time (when creating the store), won't it always be empty? I mean, since no actions has yet been dispatched, save would not yet have been called, therefor localStorage cannot have been populated with state. Or am I missing something fundamental?

from redux-localstorage-simple.

I've created #12 which removes this error logging. @kilkelly please have a look and merge if you agree with this sentiment. Thanks 🙂

from redux-localstorage-simple.

Hi @jesperlehtinen , thanks for your message. The error is there to inform the developer if they provide an invalid load state to the load method. However I completely agree that on first load this doesn't make sense as it won't exist anyhow. Rather than remove this warning / error I would propose one of two options to tackle this:

1. Add a boolean disableInvalidLoadWarning property to the load method's object argument. This would give a developer the option to disable this warning / error if they want to. Example:

load({ 
  states: ["user", "products"],
  namespace: "my_cool_app",
  disableInvalidLoadWarning: true
})

disableInvalidLoadWarning's default would be false

2. Rather than use console.error to display the message we could change it to the less harsh console.warn or console.info. This may not trigger some CI tests as @danielwaltz mentioned above and makes more sense as an invalid load is not an error in a strict sense i,e, application will not crash as a result.

Let me know what you guys think.

from redux-localstorage-simple.

@kilkelly thanks for the quick feedback. I agree with @danielwaltz, the config solution (with console.warn instead of error) seems pragmatic and sufficient, and does not break existing functionality.

from redux-localstorage-simple.

Hi guys, just released v2.1.1.

@jesperlehtinen I actually coded the new disableWarnings flag last night but I didn't get it committed until just now. I've essentially just done exactly as you did :) Thanks for the PR.

@danielwaltz I used the disableWarnings name instead of disableInvalidLoadWarning as I think a more generic name sounds better in this scenario. Plus it covers future warnings.

Thanks for your input and feedback guys. Please let me know if the changes worked for you!

from redux-localstorage-simple.

@kilkelly Awesome 😄I'll go ahead and close my PR. Thanks for the quick work!

from redux-localstorage-simple.

Thanks @jesperlehtinen 👍

from redux-localstorage-simple.

That's what I like to hear 👍

from redux-localstorage-simple.