Skip to content
This repository has been archived by the owner on Jan 10, 2018. It is now read-only.

bugfix(initialState): invoke initialState if it's a function #272

Closed
wants to merge 1 commit into from
Closed

bugfix(initialState): invoke initialState if it's a function #272

wants to merge 1 commit into from

Conversation

alan-agius4
Copy link

@alan-agius4 alan-agius4 commented Nov 17, 2016

When initialState is not a function it's not being invoked

#273

@alan-agius4
Copy link
Author

image

@alan-agius4
Copy link
Author

Any update please?

@shprink
Copy link
Contributor

shprink commented Feb 1, 2017

I do not think this would work with AoT.

@alan-agius4
Copy link
Author

alan-agius4 commented Feb 1, 2017

Why not? The whole problem here is that when the initialState is a function its never invoked. And since it is using useFactory its is expecting to be an invoked function

I did this in some other private libraries and always worked likea charm woth AOT.

In fact, we are hacking around this thing because ngrx has this bug.

@sebelga
Copy link

sebelga commented Mar 29, 2017

Could this be merge? I also have to add those 3 lines in my ng2.ts to make AOT work. And it does work very well but it would be nice to have it in a next release.
thanks

@alan-agius4
Copy link
Author

Bump!!!

@sebelga
Copy link

sebelga commented Apr 3, 2017

I don't understand what is the problem with this. 3 lines of code to be able to pass a function to the initical state and thus make the lib work with AOT (which does not allow a call to a lambda function).

Thus this:

StoreModule.provideStore(rootReducer, LocalstorageService.loadState()),

will not work, but this

StoreModule.provideStore(rootReducer, LocalstorageService.loadState),

works.

And this simple method allows to load the state from localStorage which is quiet handy for a "state manager" library :)

thanks

@alan-agius4
Copy link
Author

alan-agius4 commented Apr 3, 2017

@MikeRyanDev can we have some feedback please? It's been open since November!

@sebelga
Copy link

sebelga commented Apr 3, 2017

I think the update is that it can't be merge at all, the file does not exist anymore :) They are in the middle of a big refactor, there is no more StoreModule.provideStore but a StoreModule.forRoot or StoreModule.forFeature.

I am using your solution in the meantime :)

@alan-agius4
Copy link
Author

alan-agius4 commented Apr 3, 2017 via email

@sebelga
Copy link

sebelga commented Apr 4, 2017

Yeah, they could have worked on the new version on another branch, leaving the master branch available for small updates like this one. But it's ok, I am sure the new version is going to take care of a lot of things including this.

@tomazy
Copy link

tomazy commented Jun 7, 2017

@alan-agius4 isn't this a duplicate of #217?

@alan-agius4
Copy link
Author

Yes looks like it

@alan-agius4 alan-agius4 closed this Jun 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants