feat(store): add runtime check for action type uniqueness #2520
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Preview docs changes for 08e7ad7 at https://previews.ngrx.io/pr2520-08e7ad7/ |
timdeschryver
force-pushed
the
pr/runtimecheck-action-type
branch
from
302e37f
to
7a94b55
Compare
|
What do you think about using a dictionary of |
|
@brandonroberts that works! |
alex-okrushko
requested changes
May 22, 2020
modules/store/src/runtime_checks.ts
Outdated
| } | ||
|
|
||
| const duplicates = Object.entries(REGISTERED_ACTION_TYPES) | ||
| .filter(([_type, registrations]) => registrations > 1) |
There was a problem hiding this comment.
just [, registrations] if _type is unused.
modules/store/src/models.ts
Outdated
| /** | ||
| * Verifies that action types are not registered more than once | ||
| */ | ||
| strictActionTypeUniqueness: boolean; |
modules/store/src/runtime_checks.ts
Outdated
| export function _runtimeChecksFactory( | ||
| runtimeChecks: RuntimeChecks | ||
| ): RuntimeChecks { | ||
| return runtimeChecks; | ||
| } | ||
|
|
||
| export function _actionTypeUniquenessCheck(config: RuntimeChecks) { |
jsaguet
reviewed
May 27, 2020
|
🤔 now why does it format differently ... |
alex-okrushko
approved these changes
May 27, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently there is no way to warn devs when there are multiple actions with the same action type.
I've seen that this might lead to some confusion and some debugging time to spot the cause of a bug.
The most recent one, https://twitter.com/FabianGosebrink/status/1259043401724542978
The ngrx-tslint-rules has a TSLint rule to spot these, but not many people know that this exists.
What if we create a runtime check for it?
What is the new behavior?
In the PR, you see that action types are registered in a global array while the action is instantiated with
createAction. When a store module is loaded, we can perform a check to see if there are duplicates in this array, and throw an error if there are.This new feature is by default disabled, but can be enabled with the
strictActionTypeUniquenessruntime check flag.Thoughts?
Does this PR introduce a breaking change?
Other information