chore: Citations loading optimization#38837
Conversation
WalkthroughThis pull request introduces a series of type-related modifications across multiple components in the design system. The primary change involves replacing Changes
Suggested labels
Suggested reviewers
Poem
Possibly related PRs
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1)
70-73: Type guard usage is well-implemented.
Usingsize is NonNullable<IconButtonProps["size"]>makes the filter logic safer, ensuring only valid size values pass through. Consider storing["xSmall", "large"]in a constant array if these exclusions are reused elsewhere.app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (3)
79-83: Consistent approach
Repeating the size filter logic for mapping is consistent, but consider extracting this filtering logic into a small helper function for reusability.
91-95: Minor duplication
Again, the repetitive filter-mapping pattern for sizes suggests a DRY principle improvement opportunity.
103-107: DRY opportunity
Given the identical filter code, consider a reusable function or custom hook that returns the supported sizes for maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
app/client/packages/design-system/widgets/src/components/Button/src/Button.tsx(1 hunks)app/client/packages/design-system/widgets/src/components/Button/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx(2 hunks)app/client/packages/design-system/widgets/src/components/ComboBox/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx(1 hunks)app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx(2 hunks)app/client/packages/design-system/widgets/src/components/Icon/src/Icon.tsx(4 hunks)app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx(2 hunks)app/client/packages/design-system/widgets/src/components/InlineButtons/src/types.ts(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/Input/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/Modal/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/Select/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/Select/stories/Select.stories.tsx(2 hunks)app/client/packages/design-system/widgets/src/components/Spinner/src/index.tsx(1 hunks)app/client/packages/design-system/widgets/src/components/Spinner/src/styles.module.css(0 hunks)app/client/packages/design-system/widgets/src/components/TextArea/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/TextField/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/ToolbarButtons/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- app/client/packages/design-system/widgets/src/components/Spinner/src/styles.module.css
✅ Files skipped from review due to trivial changes (1)
- app/client/packages/design-system/widgets/src/components/TextArea/src/types.ts
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/widgets/src/components/Icon/src/Icon.tsx
[error] 36-36: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: chromatic-deployment
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
🔇 Additional comments (27)
app/client/packages/design-system/widgets/src/components/Icon/src/Icon.tsx (4)
1-1: LGTM! Good choice using clsx.Using clsx for className management is a standard practice for conditional class handling in React.
11-11: Props destructuring looks good.Clean destructuring with sensible defaults for size.
20-22: Type definition is properly handled.Good type safety with optional className prop for the lazy-loaded component.
35-35: Consistent className handling in both render paths.Good use of clsx to combine base styles with custom classNames.
Also applies to: 42-42
app/client/packages/design-system/widgets/src/components/Select/src/types.ts (1)
11-11: Refined type usage
Switching fromOmittoExcludeprovides clearer intent for removing specific keys from the type.app/client/packages/design-system/widgets/src/components/TextField/src/types.ts (1)
9-9: Maintain type consistency
ReplacingOmitwithExcludecontinues the trend towards more explicit type handling.app/client/packages/design-system/widgets/src/components/TimeField/src/types.ts (1)
13-13: Refine type usage
The substitution ofOmitwithExcludeensures consistent handling of omitted sizes.app/client/packages/design-system/widgets/src/components/Spinner/src/index.tsx (3)
3-3: Spot-on import
ImportingIconPropsfor direct reference is a neat approach for maintaining strongly typed props.
6-8: Clear and concise interface
DefiningSpinnerPropswith anExcludeensures "large" is explicitly omitted, matching design constraints.
10-11: Sensible default
Defaulting theSpinnersize tomediummaintains uniform consistency across the design system.app/client/packages/design-system/widgets/src/components/ComboBox/src/types.ts (1)
11-11: Use ofExcludeis Appropriate for Union Types
Switching fromOmittoExcludecorrectly removes the specified keys from a union type. This change clarifies the intent and aligns with TypeScript best practices for union type manipulation.app/client/packages/design-system/widgets/src/components/Datepicker/src/types.ts (1)
14-14: Consistent Type Definition Across Components
ApplyingExcludehere maintains consistency across the codebase. Removing"xSmall"and"large"withExcludeensures the type precisely matches the intended valid sizes.app/client/packages/design-system/widgets/src/components/Input/src/types.ts (1)
13-13: Appropriate Correction fromOmittoExclude
UsingExcludeclarifies that you're removing the"xSmall"literal from a union, rather than omitting a property from an object type. This is a beneficial refactor.app/client/packages/design-system/widgets/src/components/Button/src/types.ts (1)
43-43: Efficient Exclusion of"large"from Possible Sizes
The switch fromOmittoExcludecleanly removes the"large"key from the union, improving accuracy and readability of the button size type.app/client/packages/design-system/widgets/src/components/Datepicker/src/DatepickerTrigger.tsx (1)
12-12: Use Exclude Instead of OmitSwitching from
OmittoExcludehere improves precision for union types, ensuring unused sizes are skipped. This is consistent with the revised approach across the codebase.app/client/packages/design-system/widgets/src/components/InlineButtons/src/types.ts (1)
37-37: Exclude "large" from size propChanging
sizefromOmit<...>toExclude<...>is a neat way to keep the union consistent while explicitly disallowing “large.” Looks good.app/client/packages/design-system/widgets/src/components/Modal/src/types.ts (1)
22-22: Refine modal size typeGoing from
OmittoExcludeis a cleaner syntax for removing "xSmall" from the union, resulting in a clearer allowable sizes definition.app/client/packages/design-system/widgets/src/components/ToolbarButtons/src/types.ts (1)
36-36: Consolidate size prop typing with ExcludeBy excluding “large,” this aligns perfectly with the new type strategy, ensuring consistent usage of available sizes across components.
app/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsx (1)
9-9: Explicit type import is a good practice.
ImportingIconButtonPropsfrom@appsmith/wdsclarifies the intent of theIconButton's permissible properties.app/client/packages/design-system/widgets/src/components/Select/stories/Select.stories.tsx (1)
50-53: Type-safety improvement with a type predicate.
The use of(size): size is NonNullable<SelectProps["size"]>alongside the exclusion array helps maintain clarity and safety in the size handling.app/client/packages/design-system/widgets/src/components/InlineButtons/stories/InlineButtons.stories.tsx (1)
69-73: Consistent type exclusion approach.
Filtering out"xSmall" | "large"with theExclude<keyof typeof SIZES, "xSmall" | "large">predicate is consistent with the broader refactoring to safely define supported sizes. This is clean and readable.app/client/packages/design-system/widgets/src/components/Datepicker/stories/Datepicker.stories.tsx (2)
7-7: Explicit type imports enhance clarity.
ImportingDatePickerPropsandDateValueclarifies which types are used for the DatePicker’s props, contributing to better type safety.
47-50: Robust size filtering.
Defining a type predicate inside.filterfor validDatePickerProps<DateValue>["size"]>ensures only the supported sizes are passed through, preventing accidental usage of excluded sizes.app/client/packages/design-system/widgets/src/components/Button/src/Button.tsx (1)
60-60: Ensure type consistency for Spinner size
Passing the samesizeprop to the<Spinner>is consistent with the updated type definition for the button. However, confirm that theSpinnercomponent expects the same size variants.app/client/packages/design-system/widgets/src/components/Button/stories/Button.stories.tsx (2)
3-3: Good import usage
ImportingButtonPropshere explicitly increases type clarity for the filtering logic.
69-73: Filter logic aligns with type modifications
Excluding"large"from the size list matches the newExclude<...,"large">definition, ensuring consistent rendering of supported sizes.app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1)
73-80: Type casting ensures consistency
CastingsizeasExclude<keyof typeof SIZES, "large">explicitly matches the updated type definition and avoids runtime confusion with unsupported sizes.
| <FallbackIcon | ||
| className={styles.icon} | ||
| className={clsx(styles.icon, className)} | ||
| data-size={Boolean(size) ? size : undefined} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant Boolean calls.
The Boolean calls are unnecessary as the values will be automatically coerced to boolean.
- data-size={Boolean(size) ? size : undefined}
+ data-size={size || undefined}
- data-size={Boolean(size) ? size : undefined}
+ data-size={size || undefined}Also applies to: 45-45
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
github-actions
bot
added
the
skip-changelog
/ok-to-test tags="@tag.Anvil" <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced spinner component with dynamic size configuration - Improved type safety across multiple design system components - **Bug Fixes** - Refined size prop handling for various UI components - Updated type definitions to prevent invalid size selections - **Refactor** - Replaced `Omit` with `Exclude` for more precise type management - Updated component prop type definitions - Standardized size prop handling across design system widgets <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12947772391> > Commit: 4043446 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12947772391&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Fri, 24 Jan 2025 10:51:14 UTC <!-- end of auto-generated comment: Cypress test results -->
2 participants
/ok-to-test tags="@tag.Anvil"
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
OmitwithExcludefor more precise type managementTip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12947772391
Commit: 4043446
Cypress dashboard.
Tags:
@tag.AnvilSpec:
Fri, 24 Jan 2025 10:51:14 UTC