Skip to content

Commit

Permalink
feat(store): warn on unhandled actions
Browse files Browse the repository at this point in the history
  • Loading branch information
arturovt committed Jun 23, 2022
1 parent 65e4a9a commit 3aa3031
Show file tree
Hide file tree
Showing 7 changed files with 185 additions and 5 deletions.
4 changes: 3 additions & 1 deletion docs/advanced/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ You can provide an `NgxsModuleOptions` object as the second argument of your `Ng
- `strictContentSecurityPolicy` - Set this to `true` in order to enable support for pages where a Strict Content Security Policy has been enabled. This setting circumvent some optimisations that violate a strict CSP through the use of `new Function(...)`. (Default value is `false`)
- `executionStrategy` - An advanced option that is used to gain specific control over the way that NGXS executes code that is considered to be inside the NGXS context (ie. within `@Action` handlers) and the context under which the NGXS behaviours are observed (outside the NGXS context). These observable behaviours are: `@Select(...)`, `store.select(...)`, `actions.subscribe(...)` or `store.dispatch(...).subscribe(...)`
Developers who prefer to manually control the change detection mechanism in their application may choose to use the `NoopNgxsExecutionStrategy` which does not interfere with zones and therefore relies on the external context to handle change detection (for example: `OnPush` or the Ivy rendering engine). Developers can also choose to implement their own strategy by providing an Angular service class that implements the `NgxsExecutionStrategy` interface. The default value of `null` will result in the default strategy being used. This default strategy runs NGXS operations outside Angular's zone but all observable behaviours of NGXS are run back inside Angular's zone. (The default value is `null`)
- `warnOnUnhandledActions` - An option that determines if NGXS should log a warning into the console when some action has been dispatched, but there's no handler for this action, which likely means the action has been fired & forgotten (default value is `false`).

ngxs.config.ts

Expand All @@ -30,7 +31,8 @@ export const ngxsConfig: NgxsModuleOptions = {
},
// Execution strategy overridden for illustrative purposes
// (only do this if you know what you are doing)
executionStrategy: NoopNgxsExecutionStrategy
executionStrategy: NoopNgxsExecutionStrategy,
warnOnUnhandledActions: !environment.production
};
```

Expand Down
22 changes: 18 additions & 4 deletions packages/store/src/internal/state-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { InternalDispatchedActionResults } from '../internal/dispatcher';
import { StateContextFactory } from '../internal/state-context-factory';
import { StoreValidators } from '../utils/store-validators';
import { ensureStateClassIsInjectable } from '../ivy/ivy-enabled-in-dev-mode';
import { UnhandledActionsLogger } from '../unhandled-actions-logger/unhandled-actions-logger';

/**
* State factory class
Expand Down Expand Up @@ -129,10 +130,8 @@ export class StateFactory implements OnDestroy {
}

ngOnDestroy(): void {
// I'm using non-null assertion here since `_actionsSubscrition` will
// be 100% defined. This is because `ngOnDestroy()` cannot be invoked
// on the `StateFactory` until its initialized :) An it's initialized
// for the first time along with the `NgxsRootModule`.
// This is being non-null asserted since `_actionsSubscrition` is
// initialized within the constructor.
this._actionsSubscription!.unsubscribe();
}

Expand Down Expand Up @@ -238,6 +237,8 @@ export class StateFactory implements OnDestroy {
const type = getActionTypeFromInstance(action)!;
const results = [];

let actionHasBeenHandled = false;

for (const metadata of this.states) {
const actionMetas = metadata.actions[type];

Expand Down Expand Up @@ -287,10 +288,23 @@ export class StateFactory implements OnDestroy {
} catch (e) {
results.push(throwError(e));
}

actionHasBeenHandled = true;
}
}
}

// The `UnhandledActionsLogger` will function only during
// development and be tree-shaken in production mode.
if (
(typeof ngDevMode === 'undefined' || ngDevMode) &&
this._config.warnOnUnhandledActions &&
!actionHasBeenHandled
) {
const unhandledActionsLogger = this._injector.get(UnhandledActionsLogger);
unhandledActionsLogger.warnIfNeeded(action);
}

if (!results.length) {
results.push(of({}));
}
Expand Down
Empty file.
4 changes: 4 additions & 0 deletions packages/store/src/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,7 @@ export { NgxsExecutionStrategy } from './execution/symbols';
export { ActionType, ActionOptions } from './actions/symbols';
export { NoopNgxsExecutionStrategy } from './execution/noop-ngxs-execution-strategy';
export { StateToken } from './state-token/state-token';
export {
UnhandledActionsLogger,
ignoreActions
} from './unhandled-actions-logger/unhandled-actions-logger';
6 changes: 6 additions & 0 deletions packages/store/src/symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ export class NgxsConfig {
injectContainerState: true, // TODO: default is true in v3, will change in v4
suppressErrors: true // TODO: default is true in v3, will change in v4
};
/**
* Determines if NGXS should log a warning into the console when some action has
* been dispatched, but there's no handler for this action, which likely means the
* action has been fired & forgotten.
*/
warnOnUnhandledActions = false;

constructor() {
this.compatibility = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { ɵivyEnabled, inject, Injectable } from '@angular/core';

import { ActionType } from '../actions/symbols';
import { getActionTypeFromInstance } from '../utils/utils';
import { InitState, UpdateState } from '../actions/actions';

@Injectable({ providedIn: 'root' })
export class UnhandledActionsLogger {
/**
* These actions should be ignored by default; we can increase this
* list in the future via the `ignoreActions` method.
*/
private _ignoredActions = new Set<string>([InitState.type, UpdateState.type]);

/**
* Adds actions to the internal list of actions that should be ignored.
*/
ignoreActions(...actions: ActionType[]): void {
for (const action of actions) {
this._ignoredActions.add(action.type);
}
}

/** @internal */
warnIfNeeded(action: any): void {
const actionShouldBeIgnored = Array.from(this._ignoredActions).some(
type => type === getActionTypeFromInstance(action)
);

if (actionShouldBeIgnored) {
return;
}

action =
action.constructor && action.constructor.name !== 'Object'
? action.constructor.name
: action.type;

console.warn(
`The ${action} action has been dispatched but hasn't been handled. This may happen if the state with an action handler for this action is not registered.`
);
}
}

/**
* A helper function may be invoked within constructors to add actions that
* should be ignored. This eliminates injecting the `UnhandledActionsLogger`
* directly. Moreover, developers should guard this function with the `ngDevMode`
* variable that will allow tree-shake `UnhandledActionsLogger` and this function
* itself from the production bundle:
* ```ts
* import { RouterNavigation, RouterCancel } from '@ngxs/router-plugin';
*
* declare const ngDevMode: boolean;
*
* @Component()
* export class AppComponent {
* constructor() {
* ngDevMode && ignoreActions(RouterNavigation, RouterCancel);
* }
* }
* ```
*/
export function ignoreActions(...actions: ActionType[]): void {
if (ɵivyEnabled) {
const unhandledActionsLogger = inject(UnhandledActionsLogger);
unhandledActionsLogger.ignoreActions(...actions);
}
}
85 changes: 85 additions & 0 deletions packages/store/tests/unhandled-actions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { TestBed } from '@angular/core/testing';

import { Store, NgxsModule, UnhandledActionsLogger } from '..';

describe('Unhandled actions warnings', () => {
class FireAndForget {
static readonly type = 'Fire & forget';
}

const plainObjectAction = { type: 'Fire & Forget' };

const testSetup = (warnOnUnhandledActions = true) => {
TestBed.configureTestingModule({
imports: [NgxsModule.forRoot([], { warnOnUnhandledActions })]
});

const store = TestBed.inject(Store);
const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
const unhandledActionsLogger = TestBed.inject(UnhandledActionsLogger);
return { store, warnSpy, unhandledActionsLogger };
};

it('should not warn on unhandled actions if the config option is disabled', () => {
// Arrange
const { store, warnSpy } = testSetup(/* warnOnUnhandledActions */ false);
// Act
store.dispatch(new FireAndForget());
// Assert
try {
expect(warnSpy).not.toHaveBeenCalled();
} finally {
warnSpy.mockRestore();
}
});

describe('warn on unhandled actions', () => {
it('should warn when the action is a class', () => {
// Arrange
const { store, warnSpy } = testSetup();
const action = new FireAndForget();
// Act
store.dispatch(action);
// Assert
try {
expect(warnSpy).toHaveBeenCalledTimes(1);
expect(warnSpy).toHaveBeenCalledWith(
`The FireAndForget action has been dispatched but hasn't been handled. This may happen if the state with an action handler for this action is not registered.`
);
} finally {
warnSpy.mockRestore();
}
});

it('should warn when the action is an object', () => {
// Arrange
const { store, warnSpy } = testSetup();
// Act
store.dispatch(plainObjectAction);
// Assert
try {
expect(warnSpy).toHaveBeenCalledTimes(1);
expect(warnSpy).toHaveBeenCalledWith(
`The Fire & Forget action has been dispatched but hasn't been handled. This may happen if the state with an action handler for this action is not registered.`
);
} finally {
warnSpy.mockRestore();
}
});

it('should be possible to add custom actions which should be ignored', () => {
// Arrange
const { store, warnSpy, unhandledActionsLogger } = testSetup();
unhandledActionsLogger.ignoreActions(FireAndForget, plainObjectAction);
// Act
store.dispatch(new FireAndForget());
store.dispatch(plainObjectAction);
// Assert
try {
expect(warnSpy).not.toHaveBeenCalled();
} finally {
warnSpy.mockRestore();
}
});
});
});

0 comments on commit 3aa3031

Please sign in to comment.