Skip to content

Conversation

@Rajesh-Nagarajan-11
Copy link
Member

Notes for Reviewers

This PR addresses and resolves all outstanding ESLint lint errors across the codebase. Key changes include:

  • Reordering and cleaning up import statements to comply with eslint-plugin-import rules.
  • Replacing short-circuit expressions with explicit if statements for side effects.
  • Improving type safety and error handling in test cases.
  • Refactoring React components to use proper ref forwarding patterns.
  • Enhancing state and effect logic for better reliability and maintainability.
  • General code cleanup to ensure full compliance with the project’s ESLint configuration.

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR eliminates ESLint lint errors across the codebase to ensure full compliance with the project's ESLint configuration, including the newly added eslint-plugin-import rules.

Changes:

  • Added eslint-plugin-import dependency and configured import ordering rules
  • Reordered import statements to comply with import/first rule
  • Replaced short-circuit expressions with explicit if statements for side effects
  • Refactored React components for better type safety and component structure
  • Enhanced useEffect hooks with reference tracking and deferred state updates
  • Removed extraneous whitespace from markdown documentation files

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
package.json Added eslint-plugin-import dependency
package-lock.json Updated dependency lock with new import plugin and peer dependencies
eslint.config.js Configured import plugin and added import/first rule
src/theme/theme.ts Moved import statement to top of file
src/custom/ShareModal/ShareModal.tsx Reordered imports to comply with linting rules
src/testing/ResponsiveDataTable.test.tsx Reordered imports with React import first
system/patterns/empty-states.md Removed extraneous blank lines
src/custom/Helpers/readme.md Removed extraneous blank lines
src/custom/FlipCard/FlipCard.tsx Replaced short-circuit expressions with explicit if statements
src/actors/worker/workerfy.ts Replaced short-circuit expressions with explicit if statements
src/custom/CatalogDesignTable/CatalogDesignTable.tsx Replaced short-circuit expression with explicit if statement
src/base/Hidden/Hidden.tsx Modified ref forwarding pattern by wrapping MuiHidden in div
src/custom/CustomCatalog/CatalogCardDesignLogo.tsx Moved SvgComponent definition outside parent and added missing props
src/custom/CatalogDetail/ContentClassInfo.tsx Refactored inline ClassIcon component to direct Icon rendering
src/custom/CatalogDetail/LearningSection.tsx Enhanced useEffect with ref tracking and queueMicrotask
src/custom/CatalogDetail/ChallengesSection.tsx Enhanced useEffect with ref tracking and queueMicrotask
src/testing/routing.test.ts Updated type assertion for invalid resource type test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 22 to 28
useEffect(() => {
if (autoUpdate) {
setOpenLearning(Boolean((filteredAcademyData?.['learning-path'] ?? []).length > 0));
if (autoUpdate && prevDataRef.current !== filteredAcademyData) {
prevDataRef.current = filteredAcademyData;
queueMicrotask(() => {
setOpenLearning(Boolean((filteredAcademyData?.['learning-path'] ?? []).length > 0));
});
}
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The useEffect with queueMicrotask introduces unnecessary complexity. Using queueMicrotask to defer state updates can lead to unexpected timing issues and may not provide the intended benefit. The reference tracking with prevDataRef and the queueMicrotask pattern could be simplified to a more straightforward dependency check. Consider if the queueMicrotask is truly necessary or if a simpler effect implementation would suffice.

Copilot uses AI. Check for mistakes.
Comment on lines 22 to 28
useEffect(() => {
if (autoUpdate) {
setOpenChallenges((filteredAcademyData?.['challenges'] ?? []).length > 0);
if (autoUpdate && prevDataRef.current !== filteredAcademyData) {
prevDataRef.current = filteredAcademyData;
queueMicrotask(() => {
setOpenChallenges((filteredAcademyData?.['challenges'] ?? []).length > 0);
});
}
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The useEffect with queueMicrotask introduces unnecessary complexity. Using queueMicrotask to defer state updates can lead to unexpected timing issues and may not provide the intended benefit. The reference tracking with prevDataRef and the queueMicrotask pattern could be simplified to a more straightforward dependency check. Consider if the queueMicrotask is truly necessary or if a simpler effect implementation would suffice.

Copilot uses AI. Check for mistakes.
Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Rajesh Nagarajan <139469505+Rajesh-Nagarajan-11@users.noreply.github.com>
@Rajesh-Nagarajan-11
Copy link
Member Author

I will verify the Balance Co-Pilot review and proceed to finalize this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant