migration b4 #861
migration b4 #861
Conversation
WalkthroughThe pull request focuses on enhancing type safety and consistency across multiple UI component story files. The changes primarily involve adding React and TypeScript imports, updating function signatures with explicit type annotations, and modifying render methods in story files for components like Separator, Skeleton, Strong, Switch, and Table. These modifications aim to improve type checking and provide more precise type definitions for component props and rendering functions. Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/components/ui/Switch/stories/Switch.stories.tsx (1)
Line range hint
21-23: Fix incorrect prop spreading in Switch components.The defaultChecked prop is incorrectly receiving the entire args object.
- <Switch defaultChecked={args} key={index} variant={variant} onChange={handleChange} {...args}/> + <Switch key={index} variant={variant} onChange={handleChange} {...args}/>🧰 Tools
🪛 ESLint
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 4-4: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
🧹 Nitpick comments (5)
src/components/ui/Strong/stories/Strong.stories.tsx (1)
4-4: Remove duplicate React import.The React import is duplicated. Since React 17+, the JSX transform doesn't require explicit React imports.
-import React from 'react';src/components/ui/Table/stories/Table.stories.tsx (1)
4-4: Remove unnecessary React import.The React import is not needed with the new JSX transform (React 17+).
-import React from 'react';src/components/ui/Separator/stories/Separator.stories.tsx (1)
11-11: Simplify type annotation.The type annotation can be simplified to just use SeparatorProps.
- render: (args: JSX.IntrinsicAttributes & SeparatorProps) => <SandboxEditor> + render: (args: SeparatorProps) => <SandboxEditor>src/components/ui/Switch/stories/Switch.stories.tsx (1)
9-9: Simplify type annotations.The type annotations can be simplified:
- Remove JSX.IntrinsicAttributes from render
- handleChange type can be simplified to just boolean
- render: (args: JSX.IntrinsicAttributes & SwitchProps) => <CheckBox {...args}/> + render: (args: SwitchProps) => <CheckBox {...args}/> const CheckBox = (args: SwitchProps) => { // ... - const handleChange = (state: boolean | ((prevState: boolean) => boolean)) => { + const handleChange = (state: boolean) => {Also applies to: 12-12, 16-16
src/components/ui/Skeleton/stories/Skeleton.stories.tsx (1)
Line range hint
63-67: Consider removing unused args configuration.The
Allexport defines args that aren't being used by the render function anymore. This could be confusing for other developers.export const All = { - args: { - className: '' - } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/ui/Separator/stories/Separator.stories.tsx(1 hunks)src/components/ui/Skeleton/stories/Skeleton.stories.tsx(1 hunks)src/components/ui/Strong/stories/Strong.stories.tsx(1 hunks)src/components/ui/Switch/stories/Switch.stories.tsx(1 hunks)src/components/ui/Table/stories/Table.stories.tsx(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/ui/Separator/stories/Separator.stories.tsx
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 2-2: A space is required after '{'.
(object-curly-spacing)
[error] 2-2: A space is required before '}'.
(object-curly-spacing)
[error] 4-4: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
src/components/ui/Switch/stories/Switch.stories.tsx
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 4-4: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
src/components/ui/Strong/stories/Strong.stories.tsx (1)
10-10: LGTM! Simplified render function.Removing the unused
argsparameter improves code clarity.src/components/ui/Skeleton/stories/Skeleton.stories.tsx (1)
11-11: LGTM! Simplified render function signature.The removal of unused args parameter improves code clarity.
|
|
||
| export default { | ||
| title: 'WIP/Table', | ||
| component: Table | ||
| }; | ||
|
|
||
| // More on writing stories with args: https://storybook.js.org/docs/react/writing-stories/args | ||
| const Template = (args) => { | ||
| const Template = (args: any) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any type with proper TableProps type.
Using any type defeats TypeScript's type safety. Consider importing and using the proper props type.
-const Template = (args: any) => {
+import { TableProps } from '../Table';
+const Template = (args: TableProps) => {Committable suggestion skipped: line range outside the PR's diff.
| import { JSX, ClassAttributes, HTMLAttributes } from 'react'; | ||
| import Separator, {SeparatorProps} from '../Separator'; | ||
| import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor'; | ||
| import React from 'react'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate and clean up React imports.
There are multiple redundant imports and unused types.
-import { JSX, ClassAttributes, HTMLAttributes } from 'react';
import Separator, {SeparatorProps} from '../Separator';
import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor';
-import React from 'react';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { JSX, ClassAttributes, HTMLAttributes } from 'react'; | |
| import Separator, {SeparatorProps} from '../Separator'; | |
| import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor'; | |
| import React from 'react'; | |
| import Separator, {SeparatorProps} from '../Separator'; | |
| import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor'; |
🧰 Tools
🪛 ESLint
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 2-2: A space is required after '{'.
(object-curly-spacing)
[error] 2-2: A space is required before '}'.
(object-curly-spacing)
[error] 4-4: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
| import { JSX, useState } from 'react'; | ||
| import Switch, { SwitchProps } from '../Switch'; | ||
| import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor'; | ||
| import React from 'react'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate React imports.
Multiple React imports can be consolidated.
-import { JSX, useState } from 'react';
import Switch, { SwitchProps } from '../Switch';
import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor';
-import React from 'react';Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 1-1: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
[error] 4-4: '/home/jailuser/git/node_modules/react/index.js' imported multiple times.
(import/no-duplicates)
kotAPI
added
the
automerge
Summary by CodeRabbit
Refactor
Chores