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.

Sign up for GitHub

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

Jump to bottom

feat: Add the possibility to specify action types to invoke the reducer in the storage sync helper #1320

Open
wants to merge 2 commits into
base: release/9.6
Choose a base branch
from
Open

feat: Add the possibility to specify action types to invoke the reducer in the storage sync helper #1320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97a95a0
feat: Add the possibility to specify action types to invoke the reduc…
Feb 6, 2024
12e9f0a
Merge branch 'release/9.6' into fix/store-sync
Abhirocks889 Mar 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/@o3r/store-sync/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"@angular-devkit/schematics": "~16.2.0",
"@ngrx/entity": "~16.3.0",
"@ngrx/store": "~16.3.0",
"@ngrx/store-devtools": "~16.3.0",
Abhirocks889 marked this conversation as resolved.
Show resolved Hide resolved
"@o3r/core": "workspace:^",
"@o3r/dev-tools": "workspace:^",
"@o3r/logger": "workspace:^",
Expand Down Expand Up @@ -57,6 +58,7 @@
"@angular/platform-browser-dynamic": "~16.2.0",
"@ngrx/entity": "~16.3.0",
"@ngrx/store": "~16.3.0",
"@ngrx/store-devtools": "~16.3.0",
"@nx/eslint-plugin": "~16.10.0",
"@nx/js": "~16.10.0",
"@nx/linter": "~16.10.0",
Expand Down
14 changes: 13 additions & 1 deletion packages/@o3r/store-sync/src/sync-storage/storage-sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { INIT, UPDATE } from '@ngrx/store';
import type { RECOMPUTE } from '@ngrx/store-devtools';
import { deepFill } from '@o3r/core';
import type { Logger } from '@o3r/core';
import type { StorageKeyConfiguration, StorageKeys, SyncStorageConfig, SyncStorageSyncOptions } from './interfaces';

/** The recompute action type of ngrx store devtools */
const RECOMPUTE_ACTION: typeof RECOMPUTE = '@ngrx/store-devtools/recompute';

/**
* Reviver the date from a JSON field if the string is matching iso format. Return the same value otherwise
*
Expand Down Expand Up @@ -319,6 +323,12 @@ export const syncStorage = (config: SyncStorageConfig) => (reducer: any) => {
const logger = config.logger || console;

const stateKeys = validateStateKeys(config.keys);
const syncForFeature = stateKeys.some((key) => {
if (typeof key === 'object') {
const currentKey = Object.keys(key)[0];
return !!(key?.[currentKey] as SyncStorageSyncOptions)?.syncForFeature;
}
});
const rehydratedState = config.rehydrate
? rehydrateApplicationState(stateKeys, config.storage, config.storageKeySerializer, config.restoreDates, logger)
: undefined;
Expand All @@ -328,7 +338,9 @@ export const syncStorage = (config: SyncStorageConfig) => (reducer: any) => {

// If state arrives undefined, we need to let it through the supplied reducer
// in order to get a complete state as defined by user
if (action.type === INIT && !state) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should check syncForFeature when action.type is not INIT

I don't understand why we need RECOMPUTE
because UPDATE will be called when loading the lazy loaded store, and it should be enough no ?
RECOMPUTE seems to be for devtools so in production mode it will not be called no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the check. Also, for the second question, we need RECOMPUTE because in dev mode, we get an undefined state for both UPDATE and RECOMPUTE action types even if the reducer is invoked for UPDATE, the state arrives as undefined for RECOMPUTE. Hence, the app breaks and is not usable.

Please check the reproduction: https://github.com/Abhirocks889/store-sync-error

const checkForUpdateAndRecompute = syncForFeature && (action.type === UPDATE || action.type === RECOMPUTE_ACTION);

if ((action.type === INIT || checkForUpdateAndRecompute) && !state) {
nextState = reducer(state, action);
} else {
nextState = { ...state };
Expand Down
2 changes: 2 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8834,6 +8834,7 @@ __metadata:
"@angular/platform-browser-dynamic": "npm:~16.2.0"
"@ngrx/entity": "npm:~16.3.0"
"@ngrx/store": "npm:~16.3.0"
"@ngrx/store-devtools": "npm:~16.3.0"
"@nx/eslint-plugin": "npm:~16.10.0"
"@nx/js": "npm:~16.10.0"
"@nx/linter": "npm:~16.10.0"
Expand Down Expand Up @@ -8872,6 +8873,7 @@ __metadata:
"@angular-devkit/schematics": ~16.2.0
"@ngrx/entity": ~16.3.0
"@ngrx/store": ~16.3.0
"@ngrx/store-devtools": ~16.3.0
"@o3r/core": "workspace:^"
"@o3r/dev-tools": "workspace:^"
"@o3r/logger": "workspace:^"
Expand Down