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

stock-market refresh produce error stock undefined. #315

Closed
cloudstr21 opened this issue Aug 28, 2018 · 13 comments
Closed

stock-market refresh produce error stock undefined. #315

cloudstr21 opened this issue Aug 28, 2018 · 13 comments
Labels

Comments

@cloudstr21
Copy link

Hi there,

I founded an issue when i open examples page => stock-market sub page.
How to reproduce the issue?

  • open stock market page
  • refresh the page
  • an error will occurred, it is says "stock undefined" as shown picture below.
    issue ngrx

Thanks

@timdeschryver
Copy link
Collaborator

Seems like you're right, thanks for letting us know!
Do you want to open up a PR with a fix?

@tomastrajan
Copy link
Owner

@cloudstr21 @timdeschryver this only happens if navigating / refreshing directly on the stock-market url, probably is a race condition with initializing state from local storage / reducer initial state and expecting that state in the StockMarketComponent ngOnInit method.

@cloudstr21
Copy link
Author

Sure. I will make it as soon as I found the solutions.

@timdeschryver
Copy link
Collaborator

Awesome, let us know if we can help in any way possible.

@cloudstr21
Copy link
Author

I just create the pull request, please take a review.

@cloudstr21
Copy link
Author

this is strange, select data from ngrx/store is always undefined after page is refreshing. @timdeschryver
i tried to set timeout, but the result is the same.
this.store.pipe(select(selectStockMarket)). selector, effects, etc is fine, and this error doesn't happens in todos component.

@timdeschryver
Copy link
Collaborator

I'll take a look at it later today.

@timdeschryver
Copy link
Collaborator

timdeschryver commented Aug 30, 2018

The problem lies with the initStateFromLocalStorage meta-reducers.
Our state shape looks like the following:

{
  todos: {…}, 
  stockMarket: {…}
}

But when we init our app via initStateFromLocalStorage, we get the following state:

{
  todos: {…}, 
  stocks: {…} // this should be stockMarket
}

@timdeschryver
Copy link
Collaborator

To fix this we have two choices:

  • rename stockMarket to stocks
  • rename modify STOCK_MARKET_KEY in stock-maerket.effects to be EXAMPLES.STOCKMARKET - but this still won't work because in LocalStorageService.loadInitialState it will transform this string to lowercase, making it stockmarket instead of stockMarket.

@timdeschryver
Copy link
Collaborator

I forgot to mention, that I would vote to rename stockMarket to stocks

@cloudstr21
Copy link
Author

That's it. you are great @timdeschryver . I just fix it as your suggestions. now it run perfectly in my local. Should i re-commit it?

@tomastrajan
Copy link
Owner

@timdeschryver @cloudstr21 ah god! Sorry for this, I forgot about the relation between the local storage key and the state key. It was implemented really loooong time a ago... Great catch! We can maybe think about documenting it or reworking it so that the state key is used automatically and doesn't have to be defined twice :)

@timdeschryver
Copy link
Collaborator

@cloudstr21 Ofcourse you can re-commit it 👍

@tomastrajan No problem 😄 we could perhaps rework it as you mentioned or we could use ngrx-local-storage, which is a popular library that would do this for you. I haven't used it before tho. We can open up a new issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants