(feat) Implement config utils in the modules with config #1321
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
annacv
changed the title
CachedaCodes
suggested changes
Oct 11, 2023
Comment on lines
43
to
48
| /** | ||
| * Sets the {@link EmpathizeState.config } config. | ||
| * | ||
| * @param config - The new config. | ||
| */ | ||
| setConfig(config: EmpathizeConfig): void; |
There was a problem hiding this comment.
This type already exists in the config-store.utils.ts. You have to import the mutations type and make the mutations extend that type.
There was a problem hiding this comment.
Hi, regarding this, if we extend the mutations we shouldn't need to declare them here, so we could remove setConfig & mergeConfig types from here. If this is okay, why are we keeping setQuery mutation types in modules if we are extending QueryMutations??
| @@ -14,7 +15,8 @@ export const empathizeXStoreModule: EmpathizeXStoreModule = { | |||
| mutations: { | |||
| setIsOpen(state, isOpen) { | |||
| state.isOpen = isOpen; | |||
| } | |||
| }, | |||
| setConfig | |||
There was a problem hiding this comment.
You're missing the mergeConfig mutation.
BREAKING CHANGE: setTaggingConfig mutation has been renamed/replaced by setConfig EMP-2328
BREAKING CHANGE: setFacetsConfig mutation has been renamed/replaced by setConfig EMP-2328
BREAKING CHANGE: setPageSize mutation has been renamed/replaced by setConfig EMP-2328
…hese mutations outside the modules EMP-2328
annacv
force-pushed
the
feature/EMP-2328-Implement-config-utils
branch
from
0df2737
to
9a39dc5
Compare
BREAKING CHANGE: setPageSize mutation and setTaggingConfig mutation have been replaced with setConfig and mergeConfig mutations EMP-2328
CachedaCodes
added a commit
that referenced
this pull request
Oct 12, 2023
…)" This reverts commit 393d758.
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.
EMP-2328
BREAKING CHANGES:
setTaggingConfigmutation has been renamed/replaced bysetConfigsetFacetsConfigmutation has been renamed/replaced bysetConfigsetPageSizemutation has been renamed/replaced bysetConfigPull request template
A new store util has been created. This PR uses this config store util in the modules that have a config, no matter if they are actually doing it.
Motivation and context
The aim of this task is to enable modifying the module's config from outside them (e.g., via the experience controls module).
Type of change
What is the destination branch of this PR?
MainHow has this been tested?