fix(storage-plugin): fix undefined localStorage error during SSR (#1118)

[Dav1dde]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/ngxs/store/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #1118

What is the new behavior?

Storage during SSR is emulated with an empty in memory storage engine, which can still be modified by the user by providing a different storage engine.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

[Dav1dde]

This fixes the mentioned issue, I am just not sure if this is a solution you guys are happy with. Please give me a short review if you have other ideas of fixing the issue.

@splincode @arturovt

[Dav1dde]

Regarding tests I'd also welcome your input, adding the storage module to app.server.module.ts or moving it from app.browser.module.ts to app.module.ts should be enough?

I unfortunately cannot run the SSR tests locally, so it's hard for me to just try it out.

[Dav1dde]

@arturovt Test Repository: https://github.com/Dav1dde/ngxs-pr-1119-test

[arturovt]

@Dav1dde I believe you and can't check right now, but the InMemoryStorageEngine should be removed. Many users will receive different states because the storage will be the same for all consumers. Also add a check inside handle method.

[Dav1dde]

So we would return null in engineFactory:

if (isPlatformServer(platformId)) {
  return null: // new InMemoryStorageEngine();
}

And additionally skip the plugin when platform = server?

---

Out of curiosity, I was under the impression that angular universal has to bootstrap the application for every request, simply because stuff like the URL changes and maybe app initializer or other services may inject the request or response. In which case this wouldn't be problematic.

[arturovt]

@Dav1dde You're right but this in memory engine seems meaningless, I don't see a purpose of it. The user reloads page - the new in memory engine is created right? Or I'm missing something.

[Dav1dde]

You're right it's basically useless if the user does not inject STORAGE_ENGINE anywhere else and does not use it otherwise and rely on it - I am not sure if that's a usecase.

PS: I'll leave all the commits in for now, I will rebase them into one once you're satisified with the PR.

[arturovt]

LGTM
Just need an approve from one more member

[Dav1dde]

Awesome, thanks for your help.

Let me know if you want the commits squashed.

[arturovt]

@Dav1dde We squash, no worries :D

[splincode]

@Dav1dde I will happy if you add plugin

NgxsStoragePluginModule.forRoot({ key: [TODOS_STORAGE_KEY] })

to app.server.module.ts

https://github.com/ngxs/store/blob/master/integration/app/app.server.module.ts

If the tests pass, then the SSR works with storage-plugin!

[Dav1dde]

@splincode done and tests seem to pass.