feat(Devtools): Support for persist, lock, pause #955

dummdidumm · 2018-04-03T07:41:23Z

Closes #853:

Devtools now support:

persist: When devtools option is clicked, state survives full page reload
lock: When activated, all actions are ignored
pause: When activated, recording of actions is paused

Fixes part of #919:

When importing a state, there is now a one second delay to give lazy loaded reducers time to instantiate.
Think of this situation: User is on a page where a reducer was lazy loaded. He wants to use the persist feature. The page is reloaded. Now the "import state" action is fired from the devtools, but the actions are replayed too soon because the lazy loaded reducer is not instantiated yet. That means, the actions are replayed with incomplete reducers. To fix this, a small delay is added before importing a state.

coveralls · 2018-04-03T07:43:19Z

Coverage decreased (-0.04%) to 88.579% when pulling 8b90cf2 on dummdidumm:master into d34b88a on ngrx:master.

brandonroberts · 2018-04-04T14:41:34Z

modules/router-store/src/router_store_module.ts

@@ -279,7 +279,10 @@ export class StoreRouterConnectingModule {

    if (this.router.url !== this.storeState[this.stateKey].state.url) {
      this.navigationTriggeredByDispatch = true;
-      this.router.navigateByUrl(this.storeState[this.stateKey].state.url);


This should be a separate PR. I don't think this will work for SSR apps also because you can't guarantee this will be finished before the next navigation starts.

dummdidumm · 2018-04-04T19:36:25Z

I removed the router-store-commit from the PR and instead opened #960 .

What do you think about the other changes?

dummdidumm · 2018-04-29T12:03:54Z

@brandonroberts any plans if/when this will be merged?

brandonroberts · 2018-04-30T18:25:33Z

I'll take a look this week. I'd like something a little more predictable for waiting for the reducers to load when importing, but we'll see.

dummdidumm · 2018-05-01T20:07:42Z

Thanks for the update!

Regarding the import and "wait on lazy loaded reducers": Since lazy loaded features/reducers are directly tied to lazy loaded modules - what about injecting the Router with @Optional() and then on import

check if the router exists
if yes, if it did navigate yet
if it did not navigate yet, it means the import is done straight after reload, so we wait for the initial router navigation to end (reducers will be loaded by then) and then import the state.
if it did already navigate, we can be sure that every module/reducer we know of at this point is already loaded, so we import the state straight away.
if no router dependency exists, we fall back to waiting for 1 second. Maybe users use a different router implementation or something like that.

I could add a commit implementing this behavior. What do you think?

Side note: This would not solve all import issues with lazy loading. What about a state history which does some navigation action and only at this point a new reducer is loaded (#919)? But I think this is far more complex to think through / implement and does not belong into this PR.

dummdidumm · 2018-05-23T13:06:28Z

@brandonroberts any updates on this? What do you think about the more predictable "wait for reducers by using Router"-proposal?

dummdidumm · 2018-07-20T10:13:18Z

Sorry to ask again but is there any timeline when (or if at all) this will be merged? Any feedback would be appreciated.

brandonroberts · 2018-07-23T14:39:06Z

Apologies for getting behind on this one. Will you rebase against master?

dummdidumm · 2018-07-24T11:53:58Z

Thanks for the update, I will do it next week.

…igation if there is one

dummdidumm · 2018-07-30T15:27:18Z

I rebased the commits and also added the suggested import router enhancement (#955 (comment)). Please have a look.

…thub.com/zalmoxisus/redux-devtools-extension/blob/master/docs/API/Arguments.md

…oaded reducers time to instantiate

…igation if there is one

dummdidumm · 2018-08-08T12:44:40Z

I resolved some new merge conflicts. The build fails but this seems to have nothing to do with the commits (bazel ran out of memory and crashed).

brandonroberts · 2018-08-08T13:05:53Z

modules/store-devtools/src/extension.ts

+      map(change => this.unwrapAction(change.payload)),
+      concatMap((action: any) => {
+        if (action.type === IMPORT_STATE) {
+          if (this.router) {


I don't think the router should be integrated here. The route may need the state we are importing, but this prevents that from happening until the navigation is over.

Maybe we can listen for any @ngrx/store/update-reducers actions and delay the import until none have been received within a span of time.

liftedActions$ retrieves the IMPORT_STATE-action before it is passed to the reducer. So as long as IMPORT_STATE is not emitted by liftedActions$, the import will not be triggered. update-reducers etc will be fired before the route is loaded because that happens on a different stream of actions (actions$), so there is no block. So in my opinion this is fine - or maybe I don't quite understand what you mean 😃

I think it's not quite clear what I mean, so I'll expand on it:

Situation 1: Router is used by user. User clicks the "import"-button of the redux devtools and manually imports a state

The added logic will not matter because router.navigated will be true (the first navigation already happened) , therefore nothing to wait on, therefore the IMPORT action is fired straight away.

Situation 2: Router is used by user. User turned on the "persist"-feature and reloads.

The added logic now matters. router.navigated will be false because the router is in the process of navigating when the action comes in. What that means is that the modules connected to that route are still getting instantiated, so for example components/reducers/effects for that route are not there yet. So if we just let the IMPORT action pass through, the reducer would import the state too soon, some actions would get ignored because the reducers handling it do not exist yet. That's why we wait until the NavigationEnd-event of the router is fired. At this point we know all the initialization logic is done.

There will be no blocking of state needed for the router because initial reducers/effects/state will be loaded during navigation. These things do not come through the liftedActions$ stream, so blocking further events there until we are finished importing is not an issue. Also something like "oh you are not logged in, I will redirect you" is fine because, well, we just import the state after that and the application will get back to the correct route.

Situation 3: Router is not used by user.

The whole router logic does not matter now, we wait 1 second just to make sure.

I understand what's going on here, but I'd still rather support this in a consistent way without the router if possible.

Idea: After the IMPORT_STATE action comes in, wait some amount of time, then switch to listening for update-actions and debounce the IMPORT_STATE action. We can inject and listen to the store actions being dispatched.

Like this?

concatMap((action: any) => { if(action.type === IMPORT_STATE) { return actions$.pipe( ofType(UPDATE), // '@ngrx/store/update-reducers' debounceTime(1000), map(() => action) ); } ...

This way, no router dependency + when there is no UPDATE within 1 second, it is assumed that everything is loaded.

I think that's pretty close, we also need to account for using the import button from the extension.

We cannot know whether the IMPORT_STATE action is fired because of the persist option turned on or because of a manual import. But that's no harm, the user just has a minimal delay (of 1 second) until the manual import starts.

Gotcha. I had debounce in my mind and not debounceTime. What happens if we never get an update action though? The example being that they import from the extension, or possible don't have any feature states.

Good point, a race condition / timeout in the likes of "there is no UPDATE action at all" that fires after 1 second is needed.
I will implement it and update the PR.

brandonroberts · 2018-08-08T13:07:55Z

modules/store-devtools/src/extension.ts

    const extensionOptions: ReduxDevtoolsExtensionConfig = {
-      instanceId: instanceId,


Why is this removed?

The instanceId was set randomly at every start, so the devtools extension did not see any connection between the state before and after the reload (ids never matched). Removing it means the devtools extension takes care of that itself. If the id is not removed on ngrx' side, the persist feature therefore would not work.

brandonroberts · 2018-08-08T13:13:56Z

@dummdidumm we are still working through the Bazel memory issues, but at long as the tests pass, its fine for now. I left you some feedback on the PR. Let me know what you think.

dummdidumm · 2018-08-13T12:45:20Z

I added you some feedback, let me know what you think.

brandonroberts · 2018-08-22T03:39:30Z

modules/store-devtools/src/extension.ts

@@ -17,6 +28,8 @@ import {
  sanitizeStates,
  unliftState,
 } from './utils';
+import { UPDATE } from '../../store/src';


This should be @ngrx/store instead of a relative import

brandonroberts reviewed Apr 4, 2018

View reviewed changes

dummdidumm force-pushed the master branch from 66d6ebc to 8bd2704 Compare April 4, 2018 19:13

dummdidumm force-pushed the master branch 2 times, most recently from 8bd2704 to 69f11c1 Compare April 13, 2018 14:46

dummdidumm mentioned this pull request Apr 14, 2018

Actions dispatched from a guard should be ignored when the time-travelling debugger is active #822

Closed

dummdidumm mentioned this pull request May 14, 2018

RFC: Effects should not be triggered when using time travel debugger #1062

Closed

dummdidumm mentioned this pull request Jul 24, 2018

How to prevent action dispatch during time travel with router-store #1196

Closed

dummdidumm force-pushed the master branch from 69f11c1 to 9e9c4f5 Compare July 30, 2018 14:43

dummdidumm added a commit to dummdidumm/platform that referenced this pull request Jul 30, 2018

ngrx#955 Make import action timing smarter: wait for first router nav…

9e9c4f5

…igation if there is one

dummdidumm added a commit to dummdidumm/platform that referenced this pull request Jul 30, 2018

ngrx#955 test fix

c2b1ed6

dummdidumm force-pushed the master branch from c2b1ed6 to d3de05a Compare July 30, 2018 15:19

dummdidumm added a commit to dummdidumm/platform that referenced this pull request Jul 30, 2018

ngrx#955 test fix

d3de05a

brandonroberts self-assigned this Jul 31, 2018

dummdidumm added 7 commits August 8, 2018 14:32

Moved config stuff to corresponding file and refined some tests

29fd07e

ngrx#853 removed instanceId generation to add support for persist

980d05b

ngrx#853 add support for lock

2a861c9

ngrx#853 support for pause recording

3266a83

ngrx#853 explicitely setting default features according to https://gi…

c250d66

…thub.com/zalmoxisus/redux-devtools-extension/blob/master/docs/API/Arguments.md

ngrx#853 more tests for lock/pause

2a72678

ngrx#853 ngrx#919 wait a little before importing state to give lazy l…

2b1ad71

…oaded reducers time to instantiate

dummdidumm added 3 commits August 8, 2018 14:34

ngrx#853 build fix

f26bfaf

ngrx#955 Make import action timing smarter: wait for first router nav…

ef1ba5f

…igation if there is one

ngrx#955 test fix

4e185bd

dummdidumm force-pushed the master branch from d3de05a to 4e185bd Compare August 8, 2018 12:38

brandonroberts requested changes Aug 8, 2018

View reviewed changes

timdeschryver mentioned this pull request Aug 10, 2018

Run effects that have {dispatch: false} during dev tools time travel. #1235

Closed

refactor(store-devtools): remove router dependency from import logic

aa9fe8e

brandonroberts requested changes Aug 22, 2018

View reviewed changes

fix(store-devtools): Fix @ngrx/store imports

8b90cf2

brandonroberts approved these changes Aug 24, 2018

View reviewed changes

brandonroberts merged commit 93fcf56 into ngrx:master Aug 24, 2018

brandonroberts mentioned this pull request Aug 24, 2018

Bug(Devtools): Import state buggy when there are lazy loaded feature reducers #919

Closed

dummdidumm mentioned this pull request Nov 13, 2019

Can't import lazy-loaded module's state from json using Redux DevTools #1326

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Devtools): Support for persist, lock, pause #955

feat(Devtools): Support for persist, lock, pause #955

dummdidumm commented Apr 3, 2018 •

edited

Loading

coveralls commented Apr 3, 2018 •

edited

Loading

brandonroberts Apr 4, 2018

dummdidumm commented Apr 4, 2018

dummdidumm commented Apr 29, 2018

brandonroberts commented Apr 30, 2018

dummdidumm commented May 1, 2018

dummdidumm commented May 23, 2018

dummdidumm commented Jul 20, 2018

brandonroberts commented Jul 23, 2018

dummdidumm commented Jul 24, 2018

dummdidumm commented Jul 30, 2018 •

edited

Loading

dummdidumm commented Aug 8, 2018

brandonroberts Aug 8, 2018

brandonroberts Aug 8, 2018

dummdidumm Aug 8, 2018 •

edited

Loading

dummdidumm Aug 15, 2018

brandonroberts Aug 20, 2018

dummdidumm Aug 20, 2018 •

edited

Loading

brandonroberts Aug 21, 2018

dummdidumm Aug 21, 2018

brandonroberts Aug 21, 2018 •

edited

Loading

dummdidumm Aug 21, 2018

brandonroberts Aug 8, 2018

dummdidumm Aug 8, 2018 •

edited

Loading

brandonroberts commented Aug 8, 2018

dummdidumm commented Aug 13, 2018

brandonroberts Aug 22, 2018

		const extensionOptions: ReduxDevtoolsExtensionConfig = {
		instanceId: instanceId,

feat(Devtools): Support for persist, lock, pause #955

feat(Devtools): Support for persist, lock, pause #955

Conversation

dummdidumm commented Apr 3, 2018 • edited Loading

coveralls commented Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

dummdidumm commented Apr 4, 2018

dummdidumm commented Apr 29, 2018

brandonroberts commented Apr 30, 2018

dummdidumm commented May 1, 2018

dummdidumm commented May 23, 2018

dummdidumm commented Jul 20, 2018

brandonroberts commented Jul 23, 2018

dummdidumm commented Jul 24, 2018

dummdidumm commented Jul 30, 2018 • edited Loading

dummdidumm commented Aug 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dummdidumm Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Situation 1: Router is used by user. User clicks the "import"-button of the redux devtools and manually imports a state

Situation 2: Router is used by user. User turned on the "persist"-feature and reloads.

Situation 3: Router is not used by user.

Choose a reason for hiding this comment

dummdidumm Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonroberts Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dummdidumm Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

brandonroberts commented Aug 8, 2018

dummdidumm commented Aug 13, 2018

Choose a reason for hiding this comment

dummdidumm commented Apr 3, 2018 •

edited

Loading

coveralls commented Apr 3, 2018 •

edited

Loading

dummdidumm commented Jul 30, 2018 •

edited

Loading

dummdidumm Aug 8, 2018 •

edited

Loading

dummdidumm Aug 20, 2018 •

edited

Loading

brandonroberts Aug 21, 2018 •

edited

Loading

dummdidumm Aug 8, 2018 •

edited

Loading