-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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: lint related changes and packaging fixes #6163
Conversation
WalkthroughThe changes in this pull request primarily involve restructuring the import paths for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
packages/utils/src/emoji.ts (2)
10-18
: Ensure consistent padding in hexadecimal to decimal conversionIn
convertHexEmojiToDecimal
, leading zeros in hexadecimal codes could be significant. Converting to decimal might omit these zeros, potentially causing incorrect codes for certain emojis.Consider padding the decimal values if necessary or documenting this behavior.
35-36
: Avoid shadowing variable names in themap
functionThe
map
function's parameteremoji
shadows the outeremoji
parameter, which can cause confusion. Consider renaming it to improve clarity..split("-") - .map((emoji) => parseInt(emoji, 10).toString(16)) + .map((emojiPart) => parseInt(emojiPart, 10).toString(16)) .join("-");packages/utils/src/color.ts (1)
1-8
: Consider adding shorthand hex supportThe type definition and implementation could be extended to support shorthand hex colors (#RGB).
Consider adding an optional boolean parameter to support shorthand hex colors:
/** * Represents an RGB color with numeric values for red, green, and blue components * @typedef {Object} TRgb * @property {number} r - Red component (0-255) * @property {number} g - Green component (0-255) * @property {number} b - Blue component (0-255) */ export type TRgb = { r: number; g: number; b: number }; // Example usage in hexToRgb: // hexToRgb("#f00", true) // supports shorthand // hexToRgb("#ff0000") // full hex
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (67)
admin/core/components/admin-sidebar/root.tsx
(1 hunks)admin/package.json
(2 hunks)packages/editor/package.json
(2 hunks)packages/editor/src/core/extensions/callout/logo-selector.tsx
(1 hunks)packages/editor/src/core/extensions/callout/utils.ts
(1 hunks)packages/eslint-config/package.json
(1 hunks)packages/helpers/helpers/emoji.helper.ts
(0 hunks)packages/helpers/helpers/index.ts
(0 hunks)packages/helpers/index.ts
(0 hunks)packages/hooks/.eslintignore
(1 hunks)packages/hooks/.eslintrc.js
(1 hunks)packages/hooks/.prettierignore
(1 hunks)packages/hooks/.prettierrc
(1 hunks)packages/hooks/package.json
(1 hunks)packages/hooks/tsconfig.json
(1 hunks)packages/ui/package.json
(2 hunks)packages/ui/src/dropdown/multi-select.tsx
(1 hunks)packages/ui/src/dropdown/single-select.tsx
(1 hunks)packages/ui/src/dropdowns/context-menu/root.tsx
(1 hunks)packages/ui/src/dropdowns/custom-menu.tsx
(1 hunks)packages/ui/src/dropdowns/custom-search-select.tsx
(3 hunks)packages/ui/src/dropdowns/custom-select.tsx
(1 hunks)packages/ui/src/emoji/emoji-icon-picker-new.tsx
(1 hunks)packages/ui/src/emoji/emoji-icon-picker.tsx
(1 hunks)packages/ui/src/tabs/tabs.tsx
(1 hunks)packages/utils/.eslintignore
(1 hunks)packages/utils/.eslintrc.js
(1 hunks)packages/utils/.prettierrc
(1 hunks)packages/utils/package.json
(2 hunks)packages/utils/src/color.ts
(1 hunks)packages/utils/src/emoji.ts
(1 hunks)packages/utils/src/index.ts
(1 hunks)packages/utils/src/string.ts
(1 hunks)packages/utils/tsconfig.json
(1 hunks)space/package.json
(1 hunks)web/.prettierignore
(1 hunks)web/app/[workspaceSlug]/(projects)/sidebar.tsx
(1 hunks)web/app/profile/sidebar.tsx
(1 hunks)web/ce/components/issues/quick-add/root.tsx
(1 hunks)web/core/components/common/logo.tsx
(1 hunks)web/core/components/core/image-picker-popover.tsx
(1 hunks)web/core/components/cycles/archived-cycles/header.tsx
(1 hunks)web/core/components/cycles/cycles-view-header.tsx
(1 hunks)web/core/components/gantt-chart/sidebar/gantt-dnd-HOC.tsx
(1 hunks)web/core/components/issues/issue-layouts/calendar/issue-block-root.tsx
(1 hunks)web/core/components/issues/issue-layouts/calendar/issue-block.tsx
(1 hunks)web/core/components/issues/issue-layouts/kanban/block.tsx
(1 hunks)web/core/components/issues/issue-layouts/list/block-root.tsx
(1 hunks)web/core/components/issues/issue-layouts/properties/labels.tsx
(1 hunks)web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx
(1 hunks)web/core/components/issues/select/label.tsx
(1 hunks)web/core/components/labels/label-block/label-item-block.tsx
(1 hunks)web/core/components/modules/archived-modules/header.tsx
(1 hunks)web/core/components/modules/module-view-header.tsx
(1 hunks)web/core/components/pages/list/search-input.tsx
(1 hunks)web/core/components/profile/sidebar.tsx
(1 hunks)web/core/components/project/header.tsx
(1 hunks)web/core/components/views/view-list-header.tsx
(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-folder.tsx
(8 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx
(6 hunks)web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx
(1 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx
(2 hunks)web/core/components/workspace/sidebar/workspace-menu.tsx
(1 hunks)web/core/hooks/use-dropdown.ts
(1 hunks)web/core/hooks/use-dynamic-dropdown.tsx
(1 hunks)web/core/store/workspace/index.ts
(0 hunks)web/package.json
(2 hunks)
💤 Files with no reviewable changes (4)
- packages/helpers/helpers/index.ts
- packages/helpers/index.ts
- packages/helpers/helpers/emoji.helper.ts
- web/core/store/workspace/index.ts
✅ Files skipped from review due to trivial changes (36)
- packages/utils/.eslintignore
- packages/hooks/.prettierignore
- packages/hooks/.prettierrc
- packages/utils/.eslintrc.js
- packages/utils/tsconfig.json
- packages/utils/src/index.ts
- packages/hooks/.eslintrc.js
- packages/utils/.prettierrc
- web/core/components/labels/label-block/label-item-block.tsx
- packages/hooks/.eslintignore
- web/core/components/modules/archived-modules/header.tsx
- web/core/components/profile/sidebar.tsx
- space/package.json
- packages/eslint-config/package.json
- web/core/hooks/use-dynamic-dropdown.tsx
- packages/utils/src/string.ts
- packages/ui/src/dropdown/single-select.tsx
- web/core/components/pages/list/search-input.tsx
- packages/hooks/package.json
- web/.prettierignore
- packages/ui/src/emoji/emoji-icon-picker.tsx
- packages/editor/src/core/extensions/callout/logo-selector.tsx
- web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx
- web/ce/components/issues/quick-add/root.tsx
- web/core/components/views/view-list-header.tsx
- packages/ui/src/tabs/tabs.tsx
- web/core/components/common/logo.tsx
- packages/ui/src/dropdown/multi-select.tsx
- packages/ui/src/emoji/emoji-icon-picker-new.tsx
- web/core/components/issues/select/label.tsx
- web/core/components/cycles/cycles-view-header.tsx
- web/core/components/cycles/archived-cycles/header.tsx
- packages/ui/src/dropdowns/custom-select.tsx
- web/core/components/gantt-chart/sidebar/gantt-dnd-HOC.tsx
- web/core/components/issues/issue-layouts/calendar/issue-block-root.tsx
- web/core/components/issues/issue-layouts/properties/labels.tsx
🔇 Additional comments (40)
web/core/components/issues/issue-layouts/kanban/block.tsx (1)
9-9
: Import statement updated appropriately
The import path for useOutsideClickDetector
has been correctly updated to @plane/hooks
, reflecting the reorganization of the module structure.
packages/hooks/tsconfig.json (1)
7-7
: Restricting TypeScript compilation to the ./src
directory
Updating the include
path to ["./src"]
ensures that only the source files are compiled, which can improve build performance and prevent unintended files from being included.
packages/utils/package.json (4)
2-2
: Package name changed to @plane/utils
Renaming the package to @plane/utils
aligns with the refactoring of helper functions into a utilities package, enhancing codebase modularity.
13-13
: Build script updated to reflect new directory structure
Changing the build entry point to ./src/index.ts
matches the updated file organization and is appropriate.
14-15
: Added linting scripts
Introducing "lint"
and "lint:errors"
scripts improves code quality by ensuring consistent linting practices across the project.
18-19
: Verify the necessity and versions of added dependencies
The dependencies "isomorphic-dompurify": "^2.16.0"
and "react": "^18.3.1"
have been added. Please ensure that:
- These packages are required for the utilities package.
- The specified versions are correct. Note that React's latest stable version is
18.2.0
.
admin/package.json (2)
17-17
: Added dependency @plane/hooks
Including @plane/hooks
reflects the refactoring of hooks into a separate package and is appropriate.
20-20
: Added dependency @plane/utils
Adding @plane/utils
aligns with the introduction of the utilities package and centralizes shared utility functions.
admin/core/components/admin-sidebar/root.tsx (1)
6-6
: LGTM: Import path update aligns with module restructuring
The change from @plane/helpers
to @plane/hooks
is consistent with the broader codebase restructuring effort.
web/core/hooks/use-dropdown.ts (1)
3-3
: LGTM: Import path update aligns with module restructuring
The change from @plane/helpers
to @plane/hooks
is consistent with the broader codebase restructuring effort.
packages/ui/package.json (2)
21-22
: LGTM: Lint script additions
The addition of lint:errors
script with --quiet
flag is appropriate for focusing on critical ESLint issues.
35-36
: LGTM: Dependency reorganization
The replacement of @plane/helpers
with @plane/hooks
and @plane/utils
aligns with the codebase reorganization strategy.
packages/editor/package.json (2)
30-30
: LGTM: Lint script addition
The addition of lint:errors
script maintains consistency with other packages.
41-41
: LGTM: Dependency update
The addition of @plane/utils
and removal of @plane/helpers
aligns with the codebase reorganization.
web/package.json (2)
33-33
: LGTM: Dependency reorganization
The replacement of @plane/helpers
with @plane/hooks
and @plane/utils
maintains consistency across packages.
Also applies to: 36-36
52-52
: Verify Next.js version update compatibility
The update from ^14.2.12
to ^14.2.20
is a minor version bump.
packages/editor/src/core/extensions/callout/utils.ts (1)
2-2
: LGTM! Import change aligns with package restructuring
The migration from @plane/helpers
to @plane/utils
is consistent with the PR objectives.
web/app/[workspaceSlug]/(projects)/sidebar.tsx (1)
5-5
: LGTM! Hook import change aligns with package restructuring
The migration of useOutsideClickDetector
from @plane/helpers
to @plane/hooks
is appropriate as it follows the convention of keeping hooks in a dedicated package.
web/core/components/project/header.tsx (1)
8-8
: LGTM! Hook import change aligns with package restructuring
The migration of useOutsideClickDetector
from @plane/helpers
to @plane/hooks
maintains consistency with the package restructuring across the codebase.
packages/ui/src/dropdowns/context-menu/root.tsx (1)
4-4
: LGTM: Import path update is consistent with package reorganization
The change from @plane/helpers
to @plane/hooks
aligns with the PR's objective of package restructuring.
web/core/components/issues/issue-layouts/calendar/issue-block.tsx (1)
9-9
: LGTM: Import path update aligns with package restructuring
The change from @plane/helpers
to @plane/hooks
is consistent with the PR's package reorganization objective.
web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (4)
6-10
: LGTM: Import grouping improves readability
The grouped imports for drag-and-drop related functionality enhance code organization.
18-18
: LGTM: Import path update aligns with package restructuring
The change from @plane/helpers
to @plane/hooks
is consistent with the PR's package reorganization objective.
105-118
: LGTM: Improved formatting in getData method
The reformatting enhances readability while maintaining the same functionality:
- Clear separation of blocked states declaration
- Better indentation for configuration object
127-129
: LGTM: Cleaner onDrop handler formatting
The formatting changes improve code readability without affecting functionality.
web/core/components/modules/module-view-header.tsx (1)
11-11
: LGTM: Import path updated correctly
The import path change from @plane/helpers
to @plane/hooks
is consistent with the PR objectives and maintains the same functionality.
web/core/components/issues/issue-layouts/list/block-root.tsx (1)
9-9
: LGTM: Import path updated correctly
The import path change from @plane/helpers
to @plane/hooks
is consistent with the PR objectives and maintains the same functionality.
packages/ui/src/dropdowns/custom-menu.tsx (1)
7-7
: LGTM: Import path updated correctly
The import path change from @plane/helpers
to @plane/hooks
is consistent with the PR objectives and maintains the same functionality.
web/core/components/workspace/sidebar/workspace-menu.tsx (1)
10-10
: LGTM: Import path updated correctly
The import path change from @plane/helpers
to @plane/hooks
aligns with the package restructuring effort.
packages/ui/src/dropdowns/custom-search-select.tsx (3)
7-7
: LGTM: Import path updated correctly
The import path change from @plane/helpers
to @plane/hooks
is consistent with the package restructuring.
100-104
: LGTM: Improved template literal formatting
The className template literal has been reformatted for better readability while maintaining the same functionality.
190-196
: LGTM: Enhanced tooltip rendering structure
The tooltip rendering logic has been reformatted for better readability and maintainability.
web/app/profile/sidebar.tsx (1)
10-10
: LGTM: Import path updated correctly
The import path change from @plane/helpers
to @plane/hooks
aligns with the package restructuring effort.
web/core/components/workspace/sidebar/favorites/favorite-folder.tsx (3)
22-22
: LGTM: Import path updated correctly
The import path for useOutsideClickDetector has been updated from @plane/helpers to @plane/hooks, aligning with the module reorganization.
97-110
: LGTM: Improved code formatting and readability
The getData function's formatting has been enhanced with proper spacing and indentation, making the code more readable while maintaining the same functionality.
145-145
: LGTM: Consistent instruction state handling
The instruction state is consistently used across the component for managing drag and drop visual indicators.
Also applies to: 149-149, 303-303
web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx (1)
8-8
: LGTM: Import path updated correctly
The import path for useOutsideClickDetector has been updated from @plane/helpers to @plane/hooks, consistent with the module reorganization.
web/core/components/core/image-picker-popover.tsx (1)
12-12
: LGTM: Import path updated correctly
The import path for useOutsideClickDetector has been updated from @plane/helpers to @plane/hooks, consistent with the module reorganization.
web/core/components/workspace/sidebar/projects-list-item.tsx (2)
27-27
: LGTM! Import path update aligns with module restructuring.
The change from @plane/helpers
to @plane/hooks
is part of the broader effort to better organize hooks into a dedicated module.
301-301
: LGTM! Good addition of a unique key prop.
The key combination ${project.id}_${URLProjectId}
ensures proper component updates when either the project or URL changes.
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/utils/src/color.ts (1)
1-8
: Consider using more precise types for RGB valuesWhile the JSDoc indicates the valid range (0-255), the type system could enforce this constraint.
Consider using a more precise type definition:
type ColorComponent = number & { readonly brand: unique symbol }; const createColorComponent = (value: number): ColorComponent => { if (value < 0 || value > 255 || !Number.isInteger(value)) throw new Error('Color component must be an integer between 0 and 255'); return value as ColorComponent; }; export type RGB = { r: ColorComponent; g: ColorComponent; b: ColorComponent; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/utils/src/color.ts
(1 hunks)
🔇 Additional comments (4)
packages/utils/src/color.ts (4)
10-19
: LGTM! Well-implemented validation function
The function correctly handles value clamping and ensures integer outputs.
21-26
: LGTM! Efficient hex conversion implementation
The function effectively combines validation, conversion, and padding in a single line while maintaining readability.
28-46
:
Add explicit error handling for invalid hex colors
Currently, the function silently returns black (0,0,0) for invalid inputs, which could mask errors in the application.
Consider throwing an error for invalid inputs:
export const hexToRgb = (hex: string): RGB => {
+ if (!hex) throw new Error('Hex color string is required');
+ if (typeof hex !== 'string') throw new Error('Hex color must be a string');
const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex.trim());
- return result
- ? {
+ if (!result) throw new Error('Invalid hex color format. Expected: #RRGGBB');
+ return {
r: parseInt(result[1], 16),
g: parseInt(result[2], 16),
b: parseInt(result[3], 16),
- }
- : { r: 0, g: 0, b: 0 };
+ };
};
48-60
:
Add input validation for RGB values
While toHex
validates individual components, we should validate the input object structure itself.
Add validation at the function level:
export const rgbToHex = ({ r, g, b }: RGB): string => {
+ if (typeof r !== 'number' || typeof g !== 'number' || typeof b !== 'number') {
+ throw new Error('RGB values must be numbers');
+ }
+ if ([r, g, b].some(value => value < 0 || value > 255 || !Number.isInteger(value))) {
+ throw new Error('RGB values must be integers between 0 and 255');
+ }
return `#${toHex(r)}${toHex(g)}${toHex(b)}`;
};
Summary by CodeRabbit
Release Notes
New Features
@plane/hooks
that provides shared React hooks.@plane/utils
.Bug Fixes
Documentation
Chores
Style