Skip to content

Commit

Permalink
fix(storage-plugin): only restore state if key matches addedStates (n…
Browse files Browse the repository at this point in the history
…gxs#1746)

* fix(storage-plugin): only restore state if key matches addedStates

* test(storage-plugin): add test that ensures that state is not restored again

* chore: update CHANGELOG.md

Co-authored-by: arturovt <[email protected]>
  • Loading branch information
rhutchison and arturovt authored Jun 21, 2021
1 parent bbf2968 commit 2f9542d
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ $ npm install @ngxs/store@dev
- Fix: Devtools Plugin - Do not connect to devtools when the plugin is disabled [#1761](https://github.com/ngxs/store/pull/1761)
- Fix: Router Plugin - Cleanup subscriptions when the root view is destroyed [#1754](https://github.com/ngxs/store/pull/1754)
- Fix: WebSocket Plugin - Cleanup subscriptions and close the connection when the root view is destroyed [#1755](https://github.com/ngxs/store/pull/1755)
- Fix: Storage Plugin - Only restore state if key matches `addedStates` [#1746](https://github.com/ngxs/store/pull/1746)
- Performance: Tree-shake errors and warnings [#1732](https://github.com/ngxs/store/pull/1732)
- Performance: Tree-shake `ConfigValidator`, `HostEnvironment` and `isAngularInTestMode` [#1741](https://github.com/ngxs/store/pull/1741)
- Performance: Tree-shake `SelectFactory` [#1744](https://github.com/ngxs/store/pull/1744)
Expand Down
4 changes: 2 additions & 2 deletions bundlesize.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@
"path": "./@ngxs/storage-plugin/fesm2015/ngxs-storage-plugin.js",
"package": "@ngxs/storage-plugin",
"target": "es2015",
"maxSize": "12.000KB",
"maxSize": "12.65KB",
"compression": "none"
},
{
"path": "./@ngxs/storage-plugin/fesm5/ngxs-storage-plugin.js",
"package": "@ngxs/storage-plugin",
"target": "es5",
"maxSize": "13.630KB",
"maxSize": "14.29KB",
"compression": "none"
},
{
Expand Down
2 changes: 1 addition & 1 deletion integrations/hello-world-ng11-ivy/bundlesize.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{
"path": "./dist-integration/main.*.js",
"target": "es2015",
"maxSize": "251.00 kB",
"maxSize": "251.05 kB",
"compression": "none"
}
]
Expand Down
2 changes: 1 addition & 1 deletion integrations/hello-world-ng12-ivy/bundlesize.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{
"path": "./dist-integration/main.*.js",
"target": "es2015",
"maxSize": "236.60 kB",
"maxSize": "236.65 kB",
"compression": "none"
}
]
Expand Down
15 changes: 12 additions & 3 deletions packages/storage-plugin/src/storage.plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,20 @@ export class NgxsStoragePlugin implements NgxsPlugin {
// transformed by the `storageOptionsFactory` function that provided token
const keys = this._options.key as string[];
const matches = actionMatcher(event);
const isInitAction = matches(InitState) || matches(UpdateState);
const isInitAction = matches(InitState);
const isUpdateAction = matches(UpdateState);
const isInitOrUpdateAction = isInitAction || isUpdateAction;
let hasMigration = false;

if (isInitAction) {
if (isInitOrUpdateAction) {
for (const key of keys) {
// We're checking what states have been added by NGXS and if any of these states should be handled by
// the storage plugin. For instance, we only want to deserialize the `auth` state, NGXS has added
// the `user` state, the storage plugin will be rerun and will do redundant deserialization.
if (isUpdateAction && event.addedStates && !event.addedStates.hasOwnProperty(key)) {
continue;
}

const isMaster = key === DEFAULT_STATE_KEY;
let val: any = this._engine.getItem(key!);

Expand Down Expand Up @@ -89,7 +98,7 @@ export class NgxsStoragePlugin implements NgxsPlugin {

return next(state, event).pipe(
tap(nextState => {
if (!isInitAction || (isInitAction && hasMigration)) {
if (!isInitOrUpdateAction || (isInitOrUpdateAction && hasMigration)) {
for (const key of keys) {
let val = nextState;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { APP_BASE_HREF } from '@angular/common';
import { Router, RouterModule } from '@angular/router';
import { BrowserModule } from '@angular/platform-browser';
import { Component, NgModule, NgZone } from '@angular/core';
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
import { freshPlatform } from '@ngxs/store/internals/testing';
import { State, NgxsModule, Store, StateToken } from '@ngxs/store';

import { NgxsStoragePluginModule } from '../../';

describe('Restore state only if key matches', () => {
beforeEach(() => {
// Caretaker note: it somehow sets `/@angular-cli-builders` as a default URL, thus when running `initialNavigation()`
// it errors that there's no route definition for the `/@angular-cli-builders`.
spyOn(Router.prototype, 'initialNavigation').and.returnValue(undefined);
});

afterEach(() => {
localStorage.clear();
});

it(
'should not deserialize the state twice if the state should not be handled by the storage plugin',
freshPlatform(async () => {
// Arrange
interface AuthStateModel {
token: string;
}

const AUTH_STATE_TOKEN = new StateToken<AuthStateModel | null>('auth');

localStorage.setItem(
AUTH_STATE_TOKEN.getName(),
JSON.stringify({
token: 'initialTokenFromLocalStorage'
})
);

@State<AuthStateModel | null>({
name: AUTH_STATE_TOKEN,
defaults: null
})
class AuthState {}

@State({
name: 'user',
defaults: {}
})
class UserState {}

@Component({ template: '' })
class UserComponent {}

@NgModule({
imports: [
RouterModule.forChild([
{
path: '',
component: UserComponent
}
]),
NgxsModule.forFeature([UserState])
],
declarations: [UserComponent]
})
class UserModule {}

@Component({ selector: 'app-root', template: '<router-outlet></router-outlet>' })
class TestComponent {}

@NgModule({
imports: [
BrowserModule,
RouterModule.forRoot([
{
path: 'user',
loadChildren: () => UserModule
}
]),
NgxsModule.forRoot([AuthState]),
NgxsStoragePluginModule.forRoot({
key: [AuthState]
})
],
declarations: [TestComponent],
bootstrap: [TestComponent],
providers: [{ provide: APP_BASE_HREF, useValue: '/' }]
})
class TestModule {}

// Act
const { injector } = await platformBrowserDynamic().bootstrapModule(TestModule);
const ngZone = injector.get(NgZone);
const router = injector.get(Router);
const store = injector.get(Store);
const subscribeSpy = jest.fn();
const subscription = store.select(AuthState).subscribe(subscribeSpy);

// Well, we're explicitly setting another value for the auth state before going to the `/user` route.
// Previously, it would've retrieved the auth state value from the local storage each time the `UpdateState`
// action is dispatched. See `storage.plugin.ts` and `!event.addedStates.hasOwnProperty(key)` expression which
// runs `continue` if the state, which has been added, shouldn't be handled by the storage plugin.
localStorage.setItem(
AUTH_STATE_TOKEN.getName(),
JSON.stringify({ token: 'manuallySetTokenToEnsureItIsNotRetrievedAgain' })
);

await ngZone.run(() => router.navigateByUrl('/user'));

// Assert
expect(subscribeSpy).toHaveBeenCalledTimes(1);
expect(subscribeSpy).toHaveBeenCalledWith({
token: 'initialTokenFromLocalStorage'
});

subscription.unsubscribe();
})
);
});

0 comments on commit 2f9542d

Please sign in to comment.