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

chore: created shared utils package and moved objectKeys function to it #35615

Merged
merged 27 commits into from
Aug 22, 2024

Conversation

AmanAgarwal041
Copy link
Contributor

@AmanAgarwal041 AmanAgarwal041 commented Aug 12, 2024

Description

Created a shared utils package to move common util functions to it. This PR currently handles the objectKeys function which was earlier being used from design-system package.
This PR also introduces a way to create your own eslint custom rule for the project. All the steps are mentioned in the Readme for the same in packages/eslint-plugin.

EE PR : https://github.com/appsmithorg/appsmith-ee/pull/4943

Fixes #35508
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.All"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10507166570
Commit: f5e0eef
Cypress dashboard.
Tags: @tag.All
Spec:


Thu, 22 Aug 2024 13:06:47 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Introduced a new utility package (@appsmith/utils) to enhance functionality across the application.
    • Added an ESLint plugin to enforce the use of objectKeys, improving coding practices.
    • Implemented Jest configuration for testing TypeScript files, facilitating a robust testing environment.
  • Bug Fixes

    • Resolved redundancy in @appsmith/utils dependencies to streamline package management.
  • Documentation

    • Created essential configuration files for the ESLint plugin and Jest, enhancing integration and usage guidelines.
  • Chores

    • Established ESLint configuration for the new utility package and improved overall code quality through consistent coding standards.

@github-actions github-actions bot added Frontend This label marks the issue or pull request to reference client code Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Product Issues related to a specific integration Query & JS Pod Issues related to the query & JS Pod Task A simple Todo skip-changelog Adding this label to a PR prevents it from being listed in the changelog labels Aug 12, 2024
@AmanAgarwal041 AmanAgarwal041 changed the title chore: created utils package and moved objectKeys function to it chore: created shared utils package and moved objectKeys function to it Aug 12, 2024
Copy link
Contributor

coderabbitai bot commented Aug 12, 2024

Warning

Rate limit exceeded

@AmanAgarwal041 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 59 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between f9ab8c9 and f5e0eef.

Walkthrough

The recent updates enhance the codebase's architecture by relocating the objectKeys utility function to the @appsmith/utils shared module, simplifying import paths across various components. Additionally, the introduction of a custom ESLint plugin enriches the linting framework, ensuring adherence to coding standards and best practices throughout the project, promoting a cohesive development environment.

Changes

Files Change Summary
app/client/package.json Added new dependencies: "@appsmith/utils": "workspace:^" and "@appsmith/eslint-plugin": "workspace:^".
app/client/.eslintrc.base.json Updated ESLint configuration to include "@appsmith" and added "plugin:@appsmith/recommended".
app/client/packages/eslint-plugin/* Established a new ESLint plugin with custom rules enforcing the use of objectKeys, including tests and Jest configuration.
app/client/packages/design-system/widgets/src/components/* Refactored import statements to source objectKeys from @appsmith/utils instead of @appsmith/wds.
app/client/src/widgets/wds/* Modified imports for objectKeys to come from @appsmith/utils, updating several widget components.

Assessment against linked issues

Objective Addressed Explanation
Move Object.keys function to shared modules (35508)
Improve frontend architecture by moving utility functions (35508)
Centralize utility function imports
Enhance code modularity and maintainability

Celebrating the Change
In code we trust, our keys align,
With appsmith-utils, our imports shine.
A modular dance, so neat and bright,
Refactoring joy brings code delight! ✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5494e8e and e3cfc58.

Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (25)
  • app/client/package.json (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/utils/index.ts (1 hunks)
  • app/client/packages/utils/.eslintrc.json (1 hunks)
  • app/client/packages/utils/index.d.ts (1 hunks)
  • app/client/packages/utils/jest.config.js (1 hunks)
  • app/client/packages/utils/package.json (1 hunks)
  • app/client/packages/utils/src/index.ts (1 hunks)
  • app/client/packages/utils/src/object/index.ts (1 hunks)
  • app/client/packages/utils/src/object/keys.test.ts (1 hunks)
  • app/client/packages/utils/src/object/keys.ts (1 hunks)
  • app/client/packages/utils/tsconfig.json (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts (1 hunks)
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
Files skipped from review due to trivial changes (21)
  • app/client/package.json
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx
  • app/client/packages/design-system/widgets/src/utils/index.ts
  • app/client/packages/utils/.eslintrc.json
  • app/client/packages/utils/index.d.ts
  • app/client/packages/utils/jest.config.js
  • app/client/packages/utils/package.json
  • app/client/packages/utils/src/index.ts
  • app/client/packages/utils/src/object/index.ts
  • app/client/packages/utils/src/object/keys.test.ts
  • app/client/packages/utils/tsconfig.json
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts
Additional comments not posted (4)
app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts (1)

1-2: Great job on updating the import path!

The import path for objectKeys has been successfully updated to the new appsmith-utils package, which enhances modularity and reusability. This is a good step towards better code organization.

app/client/packages/utils/src/object/keys.ts (1)

13-15: Excellent enhancement of type safety and export style!

Switching objectKeys to a default export and specifying the return type as Array<keyof T> improves both usability and type safety. This aligns well with TypeScript conventions and makes the function more robust.

app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1)

1-3: Well done on updating the import path!

The import path for objectKeys has been updated to use the appsmith-utils package, promoting better code organization and reusability. This is a positive step towards a cleaner architecture.

app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1)

2-3: Good job on updating the import path for objectKeys.

This change aligns with the PR objectives of centralizing utility functions in a shared module. It helps improve code organization and maintainability. Ensure that all references to objectKeys in the codebase are updated to reflect this new path.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5494e8e and e3cfc58.

Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (25)
  • app/client/package.json (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/utils/index.ts (1 hunks)
  • app/client/packages/utils/.eslintrc.json (1 hunks)
  • app/client/packages/utils/index.d.ts (1 hunks)
  • app/client/packages/utils/jest.config.js (1 hunks)
  • app/client/packages/utils/package.json (1 hunks)
  • app/client/packages/utils/src/index.ts (1 hunks)
  • app/client/packages/utils/src/object/index.ts (1 hunks)
  • app/client/packages/utils/src/object/keys.test.ts (1 hunks)
  • app/client/packages/utils/src/object/keys.ts (1 hunks)
  • app/client/packages/utils/tsconfig.json (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts (1 hunks)
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
Files skipped from review due to trivial changes (16)
  • app/client/package.json
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx
  • app/client/packages/design-system/widgets/src/utils/index.ts
  • app/client/packages/utils/.eslintrc.json
  • app/client/packages/utils/index.d.ts
  • app/client/packages/utils/package.json
  • app/client/packages/utils/src/index.ts
  • app/client/packages/utils/src/object/index.ts
  • app/client/packages/utils/tsconfig.json
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts
Additional comments not posted (9)
app/client/packages/utils/src/object/keys.test.ts (1)

1-7: Great job on the test case!

The test effectively verifies that objectKeys returns the correct keys from an object. Consider adding more test cases to cover edge cases, such as empty objects or objects with non-string keys.

app/client/packages/utils/jest.config.js (1)

1-12: The Jest configuration looks solid!

The configuration is well set up for testing TypeScript and JavaScript files. Ensure that the testRegex pattern matches all intended test files.

app/client/packages/utils/src/object/keys.ts (1)

7-15: The implementation of objectKeys is well done!

Switching to a default export is a good choice for simplifying imports. Verify that all imports of objectKeys have been updated to reflect this change.

Verification successful

All imports of objectKeys are correctly updated to default exports!

The verification confirms that the objectKeys function is imported as a default export in the test file. No other instances of incorrect imports were found. Great job on maintaining consistency across the codebase!

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Ensure all imports of `objectKeys` are updated to reflect the default export.

# Test: Search for imports of `objectKeys`. Expect: Only default imports.
rg --type ts --type js -A 2 $'import objectKeys from'

Length of output: 322

app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts (1)

4-4: Good job on updating the import path!

The import path for objectKeys has been successfully updated to use the new utilities package. This helps in centralizing utility functions, which is a great step towards better maintainability.

app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1)

3-3: Nice update on the import path!

The import path for objectKeys has been updated to the new utilities package. This centralization is beneficial for code organization and maintainability.

app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1)

3-3: Great work on updating the import path!

The import path for objectKeys has been updated to use the new utilities package. This helps in centralizing utility functions, enhancing code maintainability.

app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts (1)

1-4: Verify the objectKeys function implementation.

The import of objectKeys has been changed to appsmith-utils/src/object. Ensure that the implementation in the new location matches the expected functionality, as this might affect the behavior of the component.

app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (1)

10-10: Verify the objectKeys function implementation.

The import of objectKeys has been changed to appsmith-utils/src/object. Ensure that the implementation in the new location matches the expected functionality, as this might affect the behavior of the component.

app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts (1)

3-4: Verify the objectKeys function implementation.

The import of objectKeys has been changed to appsmith-utils/src/object. Ensure that the implementation in the new location matches the expected functionality, as this might affect the behavior of the component.

app/client/packages/utils/jest.config.js Show resolved Hide resolved
app/client/packages/utils/package.json Outdated Show resolved Hide resolved
app/client/packages/utils/package.json Outdated Show resolved Hide resolved
app/client/packages/utils/package.json Outdated Show resolved Hide resolved
app/client/packages/utils/package.json Outdated Show resolved Hide resolved
app/client/packages/utils/index.d.ts Outdated Show resolved Hide resolved
app/client/packages/utils/src/object/keys.ts Outdated Show resolved Hide resolved
@KelvinOm
Copy link
Collaborator

KelvinOm commented Aug 12, 2024

@AmanAgarwal041 Could you please add yourself and @jsartisan (as the author of the original function) to CODEOWNERS file?

@AmanAgarwal041 AmanAgarwal041 marked this pull request as draft August 13, 2024 11:03
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e3cfc58 and 5606509.

Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (22)
  • app/client/package.json (1 hunks)
  • app/client/packages/design-system/widgets/package.json (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1 hunks)
  • app/client/packages/utils/.eslintignore (1 hunks)
  • app/client/packages/utils/.prettierignore (1 hunks)
  • app/client/packages/utils/jest.config.js (1 hunks)
  • app/client/packages/utils/package.json (1 hunks)
  • app/client/packages/utils/src/index.d.ts (1 hunks)
  • app/client/packages/utils/tsconfig.json (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts (1 hunks)
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
Files skipped from review due to trivial changes (6)
  • app/client/package.json
  • app/client/packages/utils/.eslintignore
  • app/client/packages/utils/.prettierignore
  • app/client/packages/utils/jest.config.js
  • app/client/packages/utils/src/index.d.ts
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts
Files skipped from review as they are similar to previous changes (15)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx
  • app/client/packages/utils/package.json
  • app/client/packages/utils/tsconfig.json
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts
Additional comments not posted (1)
app/client/packages/design-system/widgets/package.json (1)

14-14: Good job on adding the @appsmith/utils dependency!

This addition aligns well with the goal of centralizing utility functions into a shared package, promoting better code organization and reusability. Make sure that all necessary utilities are correctly exported and utilized in the package.

@AmanAgarwal041 AmanAgarwal041 marked this pull request as ready for review August 14, 2024 09:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5606509 and 0c821d2.

Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (26)
  • app/client/.eslintrc.base.json (2 hunks)
  • app/client/package.json (2 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1 hunks)
  • app/client/packages/eslint-plugin/index.js (1 hunks)
  • app/client/packages/eslint-plugin/object-keys-replacement/rule.js (1 hunks)
  • app/client/packages/eslint-plugin/package.json (1 hunks)
  • app/client/packages/utils/jest.config.js (1 hunks)
  • app/client/packages/utils/package.json (1 hunks)
  • app/client/packages/utils/src/index.ts (1 hunks)
  • app/client/packages/utils/src/object/index.ts (1 hunks)
  • app/client/packages/utils/src/object/keys.test.ts (1 hunks)
  • app/client/packages/utils/src/object/keys.ts (1 hunks)
  • app/client/packages/utils/tsconfig.json (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts (1 hunks)
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
Files skipped from review due to trivial changes (4)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx
  • app/client/packages/eslint-plugin/package.json
  • app/client/packages/utils/jest.config.js
  • app/client/packages/utils/package.json
Files skipped from review as they are similar to previous changes (19)
  • app/client/package.json
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx
  • app/client/packages/utils/src/index.ts
  • app/client/packages/utils/src/object/index.ts
  • app/client/packages/utils/src/object/keys.test.ts
  • app/client/packages/utils/src/object/keys.ts
  • app/client/packages/utils/tsconfig.json
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts
Additional comments not posted (3)
app/client/packages/eslint-plugin/index.js (1)

1-8: Great job setting up the custom ESLint rule!

The setup correctly imports and exports the objectKeysRule. This will help enforce the use of the objectKeys function across the codebase.

app/client/packages/eslint-plugin/object-keys-replacement/rule.js (1)

1-34: Well-defined ESLint rule for objectKeys usage!

The rule correctly identifies the use of Object.keys and suggests using objectKeys instead. The message is clear and the rule is set up as a suggestion, which is appropriate for this context.

app/client/.eslintrc.base.json (1)

19-20: Nice addition of the custom plugin and rule!

The inclusion of @appsmith-plugins and the object-keys rule in the ESLint configuration is well done. This ensures that the new rule is enforced across the codebase, promoting consistency and adherence to the updated standards.

Also applies to: 88-89

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0809b7e and 7463e68.

Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (16)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx (1 hunks)
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1 hunks)
  • app/client/packages/eslint-plugin/.eslintrc.json (1 hunks)
  • app/client/packages/eslint-plugin/.prettierignore (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts (1 hunks)
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts (1 hunks)
Files skipped from review due to trivial changes (6)
  • app/client/packages/design-system/widgets/src/components/Button/chromatic/Button.chromatic.stories.tsx
  • app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx
  • app/client/packages/eslint-plugin/.eslintrc.json
  • app/client/packages/eslint-plugin/.prettierignore
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/styleConfig.ts
Files skipped from review as they are similar to previous changes (10)
  • app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx
  • app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx
  • app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx
  • app/client/src/widgets/wds/WDSButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSIconButtonWidget/config/propertyPaneConfig/styleConfig.ts
  • app/client/src/widgets/wds/WDSInlineButtonsWidget/config/propertyPaneConfig/contentConfig.ts
  • app/client/src/widgets/wds/WDSMenuButtonWidget/config/defaultsConfig.ts
  • app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/PanelConfig/General.ts
  • app/client/src/widgets/wds/WDSToolbarButtonsWidget/config/propertyPaneConfig/styleConfig.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7463e68 and 491ccbf.

Files selected for processing (1)
  • app/client/packages/eslint-plugin/src/index.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • app/client/packages/eslint-plugin/src/index.ts

@dvj1988
Copy link
Contributor

dvj1988 commented Aug 21, 2024

@AmanAgarwal041 Can we merge this? Are there any pending comments to be resolved?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 491ccbf and 75ac33d.

Files selected for processing (1)
  • app/client/packages/eslint-plugin/src/index.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • app/client/packages/eslint-plugin/src/index.ts

@AmanAgarwal041
Copy link
Contributor Author

@AmanAgarwal041 Can we merge this? Are there any pending comments to be resolved?

It was some client linting error that were coming. I have fixed those. Waiting for the test to pass.

@dvj1988
Copy link
Contributor

dvj1988 commented Aug 22, 2024

@AmanAgarwal041 Can we merge this? Are there any pending comments to be resolved?

It was some client linting error that were coming. I have fixed those. Waiting for the test to pass.

@AmanAgarwal041 Looks like all of them have passed.

KelvinOm
KelvinOm previously approved these changes Aug 22, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this local file? I think we already have everything that listed here in the main one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, actually somehow it was asking to prettify package.json and Readme.md which I added here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I checked, now the prettier should not as about package.json, everything is fine here. Also Readme.md can be fixed, prettier will make valid improvements.

Another small nuance is that readme file is usually called only in capital letters. Readme.md > README.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed readme and prettier from the file, but need the file to avoid dist prettier check. Update Readme to README.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
app/client/packages/eslint-plugin/README.md (1)

9-9: Consider adding a comma for clarity.

In the sentence, "Navigate to your Appsmith ESLint plugin directory i.e. app/client/packages/eslint-plugin," consider adding a comma after "i.e." for improved readability.

Navigate to your Appsmith ESLint plugin directory i.e. `app/client/packages/eslint-plugin`.
+Navigate to your Appsmith ESLint plugin directory, i.e., `app/client/packages/eslint-plugin`.
Tools
LanguageTool

[uncategorized] ~9-~9: Possible missing comma found.
Context: ...Navigate to your Appsmith ESLint plugin directory i.e. `app/client/packages/eslint-plugin...

(AI_HYDRA_LEO_MISSING_COMMA)

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 75ac33d and f9ab8c9.

Files selected for processing (2)
  • app/client/packages/eslint-plugin/README.md (1 hunks)
  • app/client/packages/eslint-plugin/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • app/client/packages/eslint-plugin/package.json
Additional context used
LanguageTool
app/client/packages/eslint-plugin/README.md

[uncategorized] ~9-~9: Possible missing comma found.
Context: ...Navigate to your Appsmith ESLint plugin directory i.e. `app/client/packages/eslint-plugin...

(AI_HYDRA_LEO_MISSING_COMMA)

Additional comments not posted (5)
app/client/packages/eslint-plugin/README.md (5)

40-54: Great job on clear instructions!

The steps to update the plugin index file are well-explained and easy to follow.


56-89: Excellent testing instructions!

The guide provides a solid foundation for testing custom rules with ESLint's RuleTester.


91-98: Clear guidance on client configuration!

The steps to add the custom rule to the client configuration are straightforward and easy to understand.


100-104: Valuable resources provided!

The additional resources section offers useful links for further exploration of ESLint.


106-106: Nice closing statement!

The friendly sign-off adds a welcoming touch to the guide.

@AmanAgarwal041 AmanAgarwal041 dismissed sagar-qa007’s stale review August 22, 2024 14:18

Suggestions have been addressed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) You can still delete this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KelvinOm I already merged this PR. Should we delete this file and add dist in the root .prettierignore ?

@AmanAgarwal041 AmanAgarwal041 merged commit 87d22ca into release Aug 22, 2024
92 checks passed
@AmanAgarwal041 AmanAgarwal041 deleted the chore/35508-object-keys branch August 22, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend This label marks the issue or pull request to reference client code Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Product Issues related to a specific integration ok-to-test Required label for CI Query & JS Pod Issues related to the query & JS Pod skip-changelog Adding this label to a PR prevents it from being listed in the changelog Task A simple Todo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Move Object.keys function to shared modules
6 participants