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 runtime check for action type uniqueness #2520

Merged
merged 5 commits into from
May 27, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion modules/effects/spec/types/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const compilerOptions = () => ({
moduleResolution: 'node',
target: 'es2015',
target: 'es2017',
baseUrl: '.',
experimentalDecorators: true,
paths: {
Expand Down
2 changes: 1 addition & 1 deletion modules/router-store/spec/types/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const compilerOptions = () => ({
moduleResolution: 'node',
target: 'es2015',
target: 'es2017',
baseUrl: '.',
experimentalDecorators: true,
paths: {
Expand Down
51 changes: 50 additions & 1 deletion modules/store/spec/runtime_checks.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import * as ngCore from '@angular/core';
import { TestBed, fakeAsync, flush } from '@angular/core/testing';
import { Store, StoreModule, META_REDUCERS, USER_RUNTIME_CHECKS } from '..';
import {
Store,
StoreModule,
META_REDUCERS,
USER_RUNTIME_CHECKS,
createAction,
} from '..';
import { createActiveRuntimeChecks } from '../src/runtime_checks';
import { RuntimeChecks, Action } from '../src/models';
import * as metaReducers from '../src/meta-reducers';
import {
REGISTERED_ACTION_TYPES,
resetRegisteredActionTypes,
} from '../src/globals';

describe('Runtime checks:', () => {
describe('createActiveRuntimeChecks:', () => {
Expand All @@ -14,6 +24,7 @@ describe('Runtime checks:', () => {
strictActionImmutability: true,
strictStateImmutability: true,
strictActionWithinNgZone: false,
strictActionTypeUniqueness: false,
});
});

Expand All @@ -25,13 +36,15 @@ describe('Runtime checks:', () => {
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: true,
strictActionTypeUniqueness: true,
})
).toEqual({
strictStateSerializability: true,
strictActionSerializability: true,
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: true,
strictActionTypeUniqueness: true,
});
});

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

Expand All @@ -55,13 +69,15 @@ describe('Runtime checks:', () => {
strictStateSerializability: true,
strictActionSerializability: true,
strictActionWithinNgZone: true,
strictActionTypeUniqueness: true,
})
).toEqual({
strictStateSerializability: false,
strictActionSerializability: false,
strictActionImmutability: false,
strictStateImmutability: false,
strictActionWithinNgZone: false,
strictActionTypeUniqueness: false,
});
});
});
Expand Down Expand Up @@ -384,6 +400,39 @@ describe('Runtime checks:', () => {
});
});

describe('ActionType uniqueness', () => {
beforeEach(() => {
resetRegisteredActionTypes();
});

it('should throw when having no duplicate action types', () => {
timdeschryver marked this conversation as resolved.
Show resolved Hide resolved
createAction('action 1');
createAction('action 1');

expect(() => {
setupStore({ strictActionTypeUniqueness: true });
}).toThrowError(/Action types are registered more than once/);
});

it('should not throw when having no duplicate action types', () => {
createAction('action 1');
createAction('action 2');

expect(() => {
setupStore({ strictActionTypeUniqueness: true });
}).not.toThrowError();
});

it('should not throw when disabled', () => {
createAction('action 1');
createAction('action 1');

expect(() => {
setupStore({ strictActionTypeUniqueness: false });
}).not.toThrowError();
});
});

function setupStore(runtimeChecks?: Partial<RuntimeChecks>): Store<any> {
TestBed.configureTestingModule({
imports: [
Expand Down
2 changes: 1 addition & 1 deletion modules/store/spec/types/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const compilerOptions = () => ({
moduleResolution: 'node',
target: 'es2015',
target: 'es2017',
baseUrl: '.',
experimentalDecorators: true,
paths: {
Expand Down
3 changes: 3 additions & 0 deletions modules/store/src/action_creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
NotAllowedCheck,
Props,
} from './models';
import { REGISTERED_ACTION_TYPES } from './globals';

// Action creators taken from ts-action library and modified a bit to better
// fit current NgRx usage. Thank you Nicholas Jamieson (@cartant).
Expand Down Expand Up @@ -101,6 +102,8 @@ export function createAction<T extends string, C extends Creator>(
type: T,
config?: { _as: 'props' } | C
): ActionCreator<T> {
REGISTERED_ACTION_TYPES[type] = (REGISTERED_ACTION_TYPES[type] || 0) + 1;

if (typeof config === 'function') {
return defineType(type, (...args: any[]) => ({
...config(...args),
Expand Down
5 changes: 5 additions & 0 deletions modules/store/src/globals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export let REGISTERED_ACTION_TYPES: { [actionType: string]: number } = {};

export function resetRegisteredActionTypes() {
REGISTERED_ACTION_TYPES = {};
}
5 changes: 5 additions & 0 deletions modules/store/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,9 @@ export interface RuntimeChecks {
* Verifies that actions are dispatched within NgZone
*/
strictActionWithinNgZone: boolean;

/**
* Verifies that action types are not registered more than once
*/
strictActionTypeUniqueness: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep it optional?

}
34 changes: 34 additions & 0 deletions modules/store/src/runtime_checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import {
_ACTIVE_RUNTIME_CHECKS,
META_REDUCERS,
USER_RUNTIME_CHECKS,
_ACTION_TYPE_UNIQUENESS_CHECK,
} from './tokens';
import { REGISTERED_ACTION_TYPES } from './globals';
import { RUNTIME_CHECK_URL } from './meta-reducers/utils';

export function createActiveRuntimeChecks(
runtimeChecks?: Partial<RuntimeChecks>
Expand All @@ -22,6 +25,7 @@ export function createActiveRuntimeChecks(
strictStateImmutability: true,
strictActionImmutability: true,
strictActionWithinNgZone: false,
strictActionTypeUniqueness: false,
...runtimeChecks,
};
}
Expand All @@ -32,6 +36,7 @@ export function createActiveRuntimeChecks(
strictStateImmutability: false,
strictActionImmutability: false,
strictActionWithinNgZone: false,
strictActionTypeUniqueness: false,
};
}

Expand Down Expand Up @@ -118,8 +123,37 @@ export function provideRuntimeChecks(
];
}

export function checkForActionTypeUniqueness(): Provider[] {
return [
{
provide: _ACTION_TYPE_UNIQUENESS_CHECK,
multi: true,
deps: [_ACTIVE_RUNTIME_CHECKS],
useFactory: _actionTypeUniquenessCheck,
},
];
}

export function _runtimeChecksFactory(
runtimeChecks: RuntimeChecks
): RuntimeChecks {
return runtimeChecks;
}

export function _actionTypeUniquenessCheck(config: RuntimeChecks) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit : void :)

if (!config.strictActionTypeUniqueness) {
return;
}

const duplicates = Object.entries(REGISTERED_ACTION_TYPES)
.filter(([_type, registrations]) => registrations > 1)
Copy link
Member

Choose a reason for hiding this comment

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

just [, registrations] if _type is unused.

.map(([type]) => type);

if (duplicates.length) {
throw new Error(
`Action types are registered more than once, ${duplicates
.map(type => `"${type}"`)
.join(', ')}. ${RUNTIME_CHECK_URL}#strictactiontypeuniqueness`
);
}
}
19 changes: 16 additions & 3 deletions modules/store/src/store_module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import {
USER_PROVIDED_META_REDUCERS,
_RESOLVED_META_REDUCERS,
_ROOT_STORE_GUARD,
_ACTIVE_RUNTIME_CHECKS,
_ACTION_TYPE_UNIQUENESS_CHECK,
} from './tokens';
import { ACTIONS_SUBJECT_PROVIDERS, ActionsSubject } from './actions_subject';
import {
Expand All @@ -50,7 +52,10 @@ import {
} from './scanned_actions_subject';
import { STATE_PROVIDERS } from './state';
import { STORE_PROVIDERS, Store } from './store';
import { provideRuntimeChecks } from './runtime_checks';
import {
provideRuntimeChecks,
checkForActionTypeUniqueness,
} from './runtime_checks';

@NgModule({})
export class StoreRootModule {
Expand All @@ -61,7 +66,10 @@ export class StoreRootModule {
store: Store<any>,
@Optional()
@Inject(_ROOT_STORE_GUARD)
guard: any
guard: any,
@Optional()
@Inject(_ACTION_TYPE_UNIQUENESS_CHECK)
actionCheck: any
) {}
}

Expand All @@ -71,7 +79,10 @@ export class StoreFeatureModule implements OnDestroy {
@Inject(_STORE_FEATURES) private features: StoreFeature<any, any>[],
@Inject(FEATURE_REDUCERS) private featureReducers: ActionReducerMap<any>[],
private reducerManager: ReducerManager,
root: StoreRootModule
root: StoreRootModule,
@Optional()
@Inject(_ACTION_TYPE_UNIQUENESS_CHECK)
actionCheck: any
) {
const feats = features.map((feature, index) => {
const featureReducerCollection = featureReducers.shift();
Expand Down Expand Up @@ -166,6 +177,7 @@ export class StoreModule {
STATE_PROVIDERS,
STORE_PROVIDERS,
provideRuntimeChecks(config.runtimeChecks),
checkForActionTypeUniqueness(),
],
};
}
Expand Down Expand Up @@ -238,6 +250,7 @@ export class StoreModule {
],
useFactory: _createFeatureReducers,
},
checkForActionTypeUniqueness(),
],
};
}
Expand Down
4 changes: 4 additions & 0 deletions modules/store/src/tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,7 @@ export const _USER_RUNTIME_CHECKS = new InjectionToken<RuntimeChecks>(
export const _ACTIVE_RUNTIME_CHECKS = new InjectionToken<RuntimeChecks>(
'@ngrx/store Internal Runtime Checks'
);

export const _ACTION_TYPE_UNIQUENESS_CHECK = new InjectionToken<void>(
'@ngrx/store Check if Action types are unique'
);
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';
strictStateSerializability: true,
strictActionSerializability: true,
strictActionWithinNgZone: true,
strictActionTypeUniqueness: true,
},
}),

Expand Down
Loading