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.

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

feat(store): add strictActionWithinNgZone runtime check #2364

Merged
merged 19 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 36 additions & 0 deletions modules/store/spec/meta-reducers/inNgZoneAssert_reducer.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import * as ngCore from '@angular/core';
import { inNgZoneAssertMetaReducer } from '../../src/meta-reducers';

describe('inNgZoneAssertMetaReducer:', () => {
it('should not throw if in NgZone', () => {
ngCore.NgZone.isInAngularZone = jasmine
.createSpy('isInAngularZone')
.and.returnValue(true);
expect(() => invokeActionReducer((state: any) => state)).not.toThrow();
expect(ngCore.NgZone.isInAngularZone).toHaveBeenCalled();
});

it('should throw when not in NgZone', () => {
ngCore.NgZone.isInAngularZone = jasmine
.createSpy('isInAngularZone')
.and.returnValue(false);
expect(() => invokeActionReducer((state: any) => state)).toThrowError(
`Action 'invoke' running outside NgZone. https://ngrx.io/guide/store/configuration/runtime-checks#strictactionwithinngzone`
);
expect(ngCore.NgZone.isInAngularZone).toHaveBeenCalled();
});

it('should not call isInAngularZone when check is off', () => {
ngCore.NgZone.isInAngularZone = jasmine.createSpy('isInAngularZone');
expect(() =>
invokeActionReducer((state: any) => state, false)
).not.toThrow();
expect(ngCore.NgZone.isInAngularZone).not.toHaveBeenCalled();
});

function invokeActionReducer(reduce: Function, checkIsOn = true) {
inNgZoneAssertMetaReducer((state, action) => reduce(state, action), {
action: () => checkIsOn,
})({}, { type: 'invoke' });
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('serializationCheckMetaReducer:', () => {
payload: { foo: { bar: unSerializables['date'] } },
})
).toThrowError(
/Detected unserializable action at "payload.foo.bar.value"/
`Detected unserializable action at "payload.foo.bar.value". https://ngrx.io/guide/store/configuration/runtime-checks#strictactionserializability`
);
});
});
Expand All @@ -80,13 +80,13 @@ describe('serializationCheckMetaReducer:', () => {
).toThrowError(/Detected unserializable state at "foo.bar.value"/);
});

it('should not throw if state is null', () => {
it('should throw if state is null', () => {
expect(() => invokeStateReducer(null)).toThrowError(
/Detected unserializable state at "root"/
`Detected unserializable state at "root". https://ngrx.io/guide/store/configuration/runtime-checks#strictstateserializability`
);
});

it('should not throw if state is undefined', () => {
it('should throw if state is undefined', () => {
expect(() => invokeStateReducer(undefined)).toThrowError(
/Detected unserializable state at "root"/
);
Expand Down
63 changes: 63 additions & 0 deletions modules/store/spec/runtime_checks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('Runtime checks:', () => {
strictActionSerializability: false,
strictActionImmutability: true,
strictStateImmutability: true,
strictActionWithinNgZone: false,
});
});

Expand All @@ -23,12 +24,14 @@ describe('Runtime checks:', () => {
strictActionSerializability: true,
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: true,
})
).toEqual({
strictStateSerializability: true,
strictActionSerializability: true,
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: true,
});
});

Expand All @@ -40,6 +43,7 @@ describe('Runtime checks:', () => {
strictActionSerializability: false,
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: false,
});
});

Expand All @@ -50,12 +54,14 @@ describe('Runtime checks:', () => {
createActiveRuntimeChecks({
strictStateSerializability: true,
strictActionSerializability: true,
strictActionWithinNgZone: true,
})
).toEqual({
strictStateSerializability: false,
strictActionSerializability: false,
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: false,
});
});
});
Expand Down Expand Up @@ -88,6 +94,10 @@ describe('Runtime checks:', () => {
metaReducers,
'serializationCheckMetaReducer'
).and.callThrough();
const inNgZoneAssertMetaReducerSpy = spyOn(
metaReducers,
'inNgZoneAssertMetaReducer'
).and.callThrough();

TestBed.configureTestingModule({
imports: [StoreModule.forRoot({})],
Expand All @@ -96,13 +106,15 @@ describe('Runtime checks:', () => {
provide: USER_RUNTIME_CHECKS,
useValue: {
strictStateSerializability: false,
strictActionWithinNgZone: false,
},
},
],
});

const _store = TestBed.get<Store<any>>(Store);
expect(serializationCheckMetaReducerSpy).not.toHaveBeenCalled();
expect(inNgZoneAssertMetaReducerSpy).not.toHaveBeenCalled();
});

it('should create immutability meta reducer without config', () => {
Expand Down Expand Up @@ -320,6 +332,56 @@ describe('Runtime checks:', () => {
})
);
});

describe('Action in NgZone', () => {
const invalidAction = () => ({ type: ErrorTypes.OutOfNgZoneAction });

it(
'should throw when running outside ngZone',
fakeAsync(() => {
ngCore.NgZone.isInAngularZone = jasmine
.createSpy('isInAngularZone')
.and.returnValue(false);
const store = setupStore({ strictActionWithinNgZone: true });
expect(() => {
store.dispatch(invalidAction());
flush();
}).toThrowError(
"Action 'Action triggered outside of NgZone' running outside NgZone. https://ngrx.io/guide/store/configuration/runtime-checks#strictactionwithinngzone"
);
})
);

it(
'should not throw when running in ngZone',
fakeAsync(() => {
ngCore.NgZone.isInAngularZone = jasmine
.createSpy('isInAngularZone')
.and.returnValue(true);
const store = setupStore({ strictActionWithinNgZone: true });
expect(() => {
store.dispatch(invalidAction());
flush();
}).not.toThrowError();

expect(ngCore.NgZone.isInAngularZone).toHaveBeenCalled();
})
);

it(
'should not be called when disabled',
fakeAsync(() => {
const store = setupStore({ strictActionWithinNgZone: false });
ngCore.NgZone.isInAngularZone = jasmine.createSpy('isInAngularZone');
expect(() => {
store.dispatch(invalidAction());
flush();
}).not.toThrow();

expect(ngCore.NgZone.isInAngularZone).not.toHaveBeenCalled();
})
);
});
});

function setupStore(runtimeChecks?: Partial<RuntimeChecks>): Store<any> {
Expand All @@ -342,6 +404,7 @@ enum ErrorTypes {
UnserializableAction = 'Action type producing unserializable action',
MutateAction = 'Action type producing action mutation',
MutateState = 'Action type producing state mutation',
OutOfNgZoneAction = 'Action triggered outside of NgZone',
}

function reducerWithBugs(state: any = {}, action: any) {
Expand Down
19 changes: 19 additions & 0 deletions modules/store/src/meta-reducers/inNgZoneAssert_reducer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as ngCore from '@angular/core';
import { Action, ActionReducer } from '../models';
import { RUNTIME_CHECK_URL } from './utils';

export function inNgZoneAssertMetaReducer(
reducer: ActionReducer<any, Action>,
checks: { action: (action: Action) => boolean }
) {
return function(state: any, action: Action) {
if (checks.action(action) && !ngCore.NgZone.isInAngularZone()) {
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

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

Should we link to the docs here (and also for the realizability check)?
It can guide devs to understand what the problem is, and how to solve it.

Copy link
Author

Choose a reason for hiding this comment

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

That was my thinking behind putting the name of the check in the message.
Direct link to the docs would make that even easier.

Copy link
Member

Choose a reason for hiding this comment

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

Good idea

`Action '${
action.type
}' running outside NgZone. ${RUNTIME_CHECK_URL}#strictactionwithinngzone`
);
}
return reducer(state, action);
};
}
1 change: 1 addition & 0 deletions modules/store/src/meta-reducers/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { immutabilityCheckMetaReducer } from './immutability_reducer';
export { serializationCheckMetaReducer } from './serialization_reducer';
export { inNgZoneAssertMetaReducer } from './inNgZoneAssert_reducer';
3 changes: 2 additions & 1 deletion modules/store/src/meta-reducers/serialization_reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
isBoolean,
isString,
isArray,
RUNTIME_CHECK_URL,
} from './utils';

export function serializationCheckMetaReducer(
Expand Down Expand Up @@ -82,7 +83,7 @@ function throwIfUnserializable(

const unserializablePath = unserializable.path.join('.');
const error: any = new Error(
`Detected unserializable ${context} at "${unserializablePath}"`
`Detected unserializable ${context} at "${unserializablePath}". ${RUNTIME_CHECK_URL}#strict${context}serializability`
);
error.value = unserializable.value;
error.unserializablePath = unserializablePath;
Expand Down
3 changes: 3 additions & 0 deletions modules/store/src/meta-reducers/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export const RUNTIME_CHECK_URL =
'https://ngrx.io/guide/store/configuration/runtime-checks';

export function isUndefined(target: any): target is undefined {
return target === undefined;
}
Expand Down
5 changes: 5 additions & 0 deletions modules/store/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,9 @@ export interface RuntimeChecks {
* Verifies that actions aren't mutated
*/
strictActionImmutability: boolean;

/**
* Verifies that actions are dispatched within NgZone
*/
strictActionWithinNgZone: boolean;
}
21 changes: 21 additions & 0 deletions modules/store/src/runtime_checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { isDevMode, Provider } from '@angular/core';
import {
serializationCheckMetaReducer,
immutabilityCheckMetaReducer,
inNgZoneAssertMetaReducer,
} from './meta-reducers';
import { RuntimeChecks, MetaReducer, Action } from './models';
import {
Expand All @@ -20,6 +21,7 @@ export function createActiveRuntimeChecks(
strictActionSerializability: false,
strictStateImmutability: true,
strictActionImmutability: true,
strictActionWithinNgZone: false,
...runtimeChecks,
};
}
Expand All @@ -29,6 +31,7 @@ export function createActiveRuntimeChecks(
strictActionSerializability: false,
strictStateImmutability: false,
strictActionImmutability: false,
strictActionWithinNgZone: false,
};
}

Expand Down Expand Up @@ -64,6 +67,18 @@ function ignoreNgrxAction(action: Action) {
return action.type.startsWith('@ngrx');
}

export function createInNgZoneCheckMetaReducer({
strictActionWithinNgZone,
}: RuntimeChecks): MetaReducer {
return reducer =>
strictActionWithinNgZone
? inNgZoneAssertMetaReducer(reducer, {
action: action =>
strictActionWithinNgZone && !ignoreNgrxAction(action),
})
: reducer;
}

export function provideRuntimeChecks(
runtimeChecks?: Partial<RuntimeChecks>
): Provider[] {
Expand Down Expand Up @@ -94,6 +109,12 @@ export function provideRuntimeChecks(
deps: [_ACTIVE_RUNTIME_CHECKS],
useFactory: createSerializationCheckMetaReducer,
},
{
provide: META_REDUCERS,
multi: true,
deps: [_ACTIVE_RUNTIME_CHECKS],
useFactory: createInNgZoneCheckMetaReducer,
},
];
}

Expand Down
1 change: 1 addition & 0 deletions projects/example-app/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { AppComponent } from '@example-app/core/containers';
strictActionImmutability: true,
strictStateSerializability: true,
strictActionSerializability: true,
strictActionWithinNgZone: true,
},
}),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@

Runtime checks are here to guide developers to follow the NgRx and Redux core concepts and best practices. They are here to shorten the feedback loop of easy-to-make mistakes when you're starting to use NgRx, or even a well-seasoned developer might make. During development, when a rule is violated, an error is thrown notifying you what and where something went wrong.

`@ngrx/store` ships with four (4) built-in runtime checks:
`@ngrx/store` ships with five (5) built-in runtime checks:

- [`strictStateImmutability`](#strictstateimmutability): verifies that the state isn't mutated
- [`strictActionImmutability`](#strictactionimmutability): verifies that actions aren't mutated
- [`strictStateSerializability`](#strictstateserializability): verifies if the state is serializable
- [`strictActionSerializability`](#strictactionserializability): verifies if the actions are serializable
- Default On:
- [`strictStateImmutability`](#strictstateimmutability): verifies that the state isn't mutated.
- [`strictActionImmutability`](#strictactionimmutability): verifies that actions aren't mutated
- Default Off:
- [`strictStateSerializability`](#strictstateserializability): verifies if the state is serializable
- [`strictActionSerializability`](#strictactionserializability): verifies if the actions are serializable
- [`strictActionWithinNgZone`](#strictactionwithinngzone): verifies if actions are dispatched within NgZone

These checks are all opt-in and will automatically be disabled in production builds.
All checks will automatically be disabled in production builds.

## Enabling runtime checks
## Configuring runtime checks

It's possible to turn on the runtime checks one by one. To do so, you must enable them while providing the root store. Use the `runtimeChecks` property on the root store's config object. For each runtime check you can toggle the check with a `boolean`, `true` to enable the check, `false` to disable the check.
It's possible to override the default configuration of runtime checks. To do so, use the `runtimeChecks` property on the root store's config object. For each runtime check you can toggle the check with a `boolean`, `true` to enable the check, `false` to disable the check.

```ts
@NgModule({
Expand All @@ -24,6 +27,7 @@ It's possible to turn on the runtime checks one by one. To do so, you must enabl
strictActionImmutability: true,
strictStateSerializability: true,
strictActionSerializability: true,
strictActionWithinNgZone: true
},
}),
],
Expand Down Expand Up @@ -170,4 +174,32 @@ function logTodo (todo: Todo) {

Please note, you may not need to set `strictActionSerializability` to `true` unless you are storing/replaying actions using external resources, for example `localStorage`.

</div>
</div>

### strictActionWithinNgZone

The `strictActionWithinNgZone` check verifies that Actions are dispatched by asynchronous tasks running within `NgZone`. Actions dispatched by tasks, running outside of `NgZone`, will not trigger ChangeDetection upon completion and may result in a stale view.

Example violation of the rule:

```ts
// Callback running outside of NgZone
function callbackOutsideNgZone(){
this.store.dispatch(clearTodos());
}
```

To fix ensure actions are running within `NgZone`. Identify the event trigger and then verify if the code can be updated to use a `NgZone` aware feature. If this is not possible use the `NgZone.run` method to explicitly run the asynchronous task within NgZone.

```ts
import { NgZone } from '@angular/core';

constructor(private ngZone: NgZone){}

// Callback running outside of NgZone brought back in NgZone.
function callbackOutsideNgZone(){
this.ngZone.run(() => {
this.store.dispatch(clearTodos());
}
}
```