-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix: Added lint error for appsmith store mutations #33484
fix: Added lint error for appsmith store mutations #33484
Conversation
WalkthroughThe changes introduce a new interface Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes LGTM. Testing the DP
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9111180273. |
Deploy-Preview-URL: https://ce-33484.dp.appsmith.com |
Description
The interface we have for storing local variables using appsmith.store is immutable, meaning if we want to add variables to this, we have to use
storeValue
method to add or remove values from appsmith.store. If we use appsmith.store.value = "", we already get a linting error that saysUse storeValue() method to modify store
. Now if we are in a situation where we have stored an array inside appsmith store and we are mutating this array using push or any other array method, we should be able to see the same linting error. This Pr fixes that problem and shows error any time any function call is made on store object.Note: removed ImperativeStoreUpdate_spec cypress test case as I have added unit test case which covers this case.
Fixes #29969
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Settings"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9124146068
Commit: 56bd967
Cypress dashboard: Click here!
The following are new failures, please fix them before merging the PR:
Communication
Should the DevRel and Marketing teams inform users about this change?