chore: migrate to eslint 9 #142
Conversation
WalkthroughThis pull request introduces a comprehensive overhaul of the project's linting strategy, focusing on modernizing ESLint configuration and removing redundant linting scripts across multiple packages. The changes include introducing a new centralized Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 4
🧹 Nitpick comments (11)
packages/tailwind/build-colors.js (1)
8-9: Consider adding more robust error handling
Currently, the error is only logged to the console. Depending on your use case, you might also want to exit the process, provide a fallback directory, or re-throw the error to ensure upstream handling.packages/tailwind-formio/src/templates/stories/badge.stories.jsx (1)
15-15: Ensure consistency with the exportedargs
TheSandboxfunction no longer takesargsas a parameter. If it is no longer needed, consider removing or repurposingSandbox.argsfor clarity.packages/tailwind-formio/src/templates/stories/alert.stories.jsx (1)
15-15: Remove or utilize unused args
Similar to the badge story, if you are no longer usingargsinSandbox, removingSandbox.argsor incorporating it meaningfully elsewhere can help maintain consistency.packages/tailwind-formio/src/templates/stories/pagination.stories.jsx (1)
21-45: Use valid links or a semantic button for accessibility.Disabling
jsx-a11y/anchor-is-validis understandable for a story example, but consider replacinghref="#"with a valid route or abuttonelement to improve accessibility.packages/react-formio/src/components/table/components/defaultOperationButton.component.tsx (1)
31-33: Unused props introduced.
dataandctxare added but not used in the component. Either implement logic that uses them or remove them to avoid confusion.packages/tailwind-formio/src/templates/stories/breadcrumbs.stories.jsx (1)
42-42: Use valid links or a button element.Disabling
jsx-a11y/anchor-is-validis acceptable for demonstration, but consider using validhrefor a semantic element (button) for improved accessibility.Also applies to: 54-54, 58-58
packages/react-formio/src/components/table/components/defaultRow.component.tsx (1)
24-24: Revisit the accessibility implications of using with an interactive role.
By disabling thejsx-a11y/no-interactive-element-to-noninteractive-rolerule, there's a possibility that keyboard navigation and screen readers don’t treat this<td>as an interactive element. Consider using a more appropriate role or tag (e.g.,<button>), and ensure keyboard interactions are also supported.packages/tailwind-formio/src/templates/tailwind/button/button.stories.jsx (1)
77-77: Same comment as above regarding the removed 'args' parameter.
This change is acceptable if you're not using dynamic props in Storybook. Otherwise, ensure reintroduction or refactoring if you want to resume using Storybook Args.eslint.config.mjs (2)
1-10: Remove or keep commented-out import forvitestdeliberately.These commented imports may be confusing for newcomers, so consider either removing them if no longer needed, or adding a comment clarifying their future usage.
86-108: Commented-out Vitest-specific configuration.Similar to the earlier comment, consider removing or properly documenting this section if you do not plan to enable these tests.
packages/react-formio/src/components/react-component/reactComponent.component.tsx (1)
119-119: Unmounting React components.Removing the component via
unmountComponentAtNodeis appropriate for class-based workflows, but keep in mind that legacy methods are discouraged in newer React versions. Track and plan for any upcoming React changes if you’re considering upgrading.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
.eslintignore(0 hunks).eslintrc.cjs(0 hunks).storybook/main.ts(1 hunks).storybook/preview.ts(2 hunks)eslint.config.mjs(1 hunks)package.json(5 hunks)packages/config/package.json(0 hunks)packages/react-formio-container/package.json(0 hunks)packages/react-formio-container/src/utils/HttpClient.ts(0 hunks)packages/react-formio-container/src/views/formEdit.view.tsx(2 hunks)packages/react-formio-stores/package.json(0 hunks)packages/react-formio-stores/src/stores/auth/getAccess.action.ts(1 hunks)packages/react-formio-stores/src/stores/auth/getProjectAccess.action.ts(1 hunks)packages/react-formio-stores/src/stores/root/root.selectors.spec.ts(0 hunks)packages/react-formio/package.json(0 hunks)packages/react-formio/src/components/actions-table/actionsTable.component.spec.tsx(2 hunks)packages/react-formio/src/components/form-access/formAccess.component.tsx(0 hunks)packages/react-formio/src/components/form-action/formAction.component.tsx(1 hunks)packages/react-formio/src/components/form-control/formControl.component.tsx(1 hunks)packages/react-formio/src/components/form-edit/formEdit.reducer.ts(0 hunks)packages/react-formio/src/components/form-settings/formSettings.component.spec.tsx(4 hunks)packages/react-formio/src/components/form/form.stories.tsx(1 hunks)packages/react-formio/src/components/pagination/pagination.component.spec.tsx(0 hunks)packages/react-formio/src/components/react-component/reactComponent.component.tsx(2 hunks)packages/react-formio/src/components/select/select.component.tsx(1 hunks)packages/react-formio/src/components/table/components/defaultOperationButton.component.tsx(1 hunks)packages/react-formio/src/components/table/components/defaultRow.component.tsx(1 hunks)packages/react-formio/src/components/table/filters/defaultColumnFilter.component.tsx(1 hunks)packages/react-formio/src/react-table.d.ts(1 hunks)packages/redux-utils/package.json(0 hunks)packages/redux-utils/src/createAction.ts(1 hunks)packages/tailwind-formio/package.json(0 hunks)packages/tailwind-formio/src/templates/stories/alert.stories.jsx(1 hunks)packages/tailwind-formio/src/templates/stories/badge.stories.jsx(1 hunks)packages/tailwind-formio/src/templates/stories/breadcrumbs.stories.jsx(2 hunks)packages/tailwind-formio/src/templates/stories/pagination.stories.jsx(1 hunks)packages/tailwind-formio/src/templates/tailwind/button/button.stories.jsx(2 hunks)packages/tailwind-formio/src/templates/tailwind/index.ts(0 hunks)packages/tailwind/build-colors.js(1 hunks)packages/tailwind/package.json(0 hunks)
💤 Files with no reviewable changes (15)
- packages/tailwind-formio/src/templates/tailwind/index.ts
- packages/react-formio-container/package.json
- packages/redux-utils/package.json
- packages/react-formio/src/components/form-edit/formEdit.reducer.ts
- packages/react-formio/package.json
- packages/config/package.json
- packages/react-formio-stores/package.json
- packages/react-formio-container/src/utils/HttpClient.ts
- packages/react-formio-stores/src/stores/root/root.selectors.spec.ts
- .eslintrc.cjs
- .eslintignore
- packages/tailwind/package.json
- packages/react-formio/src/components/pagination/pagination.component.spec.tsx
- packages/tailwind-formio/package.json
- packages/react-formio/src/components/form-access/formAccess.component.tsx
✅ Files skipped from review due to trivial changes (5)
- packages/react-formio/src/components/select/select.component.tsx
- packages/react-formio/src/components/table/filters/defaultColumnFilter.component.tsx
- packages/react-formio/src/components/form-control/formControl.component.tsx
- packages/redux-utils/src/createAction.ts
- .storybook/main.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/react-formio/src/components/react-component/reactComponent.component.tsx
[error] 110-110: Do not depend on the value returned by the function ReactDOM.render().
The returned value is legacy and future versions of React might return that value asynchronously.
Check the React documentation for more information.
(lint/correctness/noRenderReturnValue)
🔇 Additional comments (26)
packages/react-formio-stores/src/stores/auth/getProjectAccess.action.ts (1)
24-24: Be cautious when discarding error details
Removing the error parameter from the catch block may obscure important debugging information if a failure occurs. If ignoring the error is intentional, consider adding a brief comment explaining the rationale, or at least log the error to facilitate future troubleshooting.
packages/react-formio-stores/src/stores/auth/getAccess.action.ts (1)
55-55: Consider retaining or logging the error object
By removing the error parameter, you risk losing essential context for diagnosing and troubleshooting. If you need to silence errors for a specific reason, consider logging them elsewhere or leaving a comment to clarify why the error is deliberately ignored.
packages/react-formio-container/src/views/formEdit.view.tsx (2)
2-2: Use of classnames is appropriate
Importing and using the classnames library is a clean way to conditionally concatenate class strings, improving flexibility without cluttering the JSX.
11-11: Integration of dynamic class names
Nicely done enabling extra styles to be passed in. Make sure to maintain consistent naming conventions for any additional classes in your components.
packages/tailwind-formio/src/templates/stories/pagination.stories.jsx (1)
15-15: Confirm removal of args parameter.
The function signature no longer includes args. Verify all invocations of Sandbox have been updated accordingly.
packages/react-formio/src/components/form-settings/formSettings.component.spec.tsx (1)
8-8: Synchronous test approach looks good.
Removing async and await is valid if no asynchronous operations are needed. Confirm there are no delayed side effects that might require await.
Also applies to: 26-26, 50-50, 62-62
packages/react-formio/src/components/form-action/formAction.component.tsx (1)
Line range hint 14-31: Consider using or removing the unused 'action' parameter and validating 'component.basePath'.
The function destructures 'action' but never uses it, even though ESLint is disabled for unused variables. Additionally, there's no check that 'component.basePath' is defined before building the URL, which may lead to runtime errors in cases where 'component.basePath' is null or undefined.
Would you like me to generate a quick script to find all references to this function and ensure that 'component.basePath' is always defined when invoked?
packages/tailwind-formio/src/templates/tailwind/button/button.stories.jsx (1)
15-15: Removed 'args' parameter may affect advanced Storybook usage.
If you still plan to pass dynamic arguments or controls to your stories, consider reintroducing the args parameter. Otherwise, this cleanup is fine.
packages/react-formio/src/components/actions-table/actionsTable.component.spec.tsx (2)
63-63: Ensure synchronous click usage remains correct.
Removing await here is fine if no asynchronous side effects are expected. Confirm that there's no need to wait for state updates or UI interactions.
76-76: Check for any asynchronous pattern that might require awaiting.
If this click triggers asynchronous processes, you might need await or waitFor to reliably test side effects. Otherwise, this change simplifies the test.
packages/react-formio/src/react-table.d.ts (2)
48-50: Consider verifying usage of totalColumnsMinWidth.
This new property adds flexibility for layout calculations. Ensure that any components leveraging this interface adjust or verify their logic to accommodate the new field.
53-55: Consider verifying usage of totalMinWidth.
Similar to totalColumnsMinWidth, confirm that columns implementing this interface properly handle or calculate minimum width requirements.
eslint.config.mjs (8)
12-15: Ignore patterns look good.
Excluding coverage and build artifacts is a common best practice that prevents clutter during linting.
16-38: Good integration of React plugin configurations.
Your usage of pluginReact.configs.flat.recommended with version detection is a modern and recommended approach for React-based projects.
39-62: Solid TypeScript integration with @typescript-eslint.
Declaring no-unused-vars as an error ensures cleaner code. Confirm that you have identified any legitimate unused variables to avoid false positives.
63-63: Accessibility plugin recommended configuration.
This is aligned with best practices to identify common accessibility pitfalls.
64-85: Enforcing React hooks rules, import sorting, and workspace checks.
Great use of these plugins to maintain code consistency and reduce future technical debt.
109-138: Thorough Testing Library configurations.
These rules provide excellent guardrails for best practices when testing your application with React Testing Library.
139-139: Prettier recommended rules.
Enforcing consistency through Prettier is a good practice to ensure a standardized code style.
140-145: Curly brace rule set to “all”.
This style-related rule helps maintain clarity in control flow by requiring explicit braces around conditional or loop bodies.
packages/react-formio/src/components/react-component/reactComponent.component.tsx (3)
107-107: Check for potential updates to the attachReact method signature.
This method now has an optional element parameter. Ensure calling code either passes the correct DOM element or handles the optional case gracefully.
116-116: detachReact signature consistency.
This method also has an optional element parameter. Confirm that you always have a valid element reference before calling it.
130-130: setValue no longer accepts flags.
Check any external calls that were passing flags to setValue. They may need to be refactored or simplified for consistent behavior.
packages/react-formio/src/components/form/form.stories.tsx (1)
125-125: Parameter removed from onAsyncSubmit.
Previously, onAsyncSubmit received submission data. Now, it’s parameterless. Confirm that upstream or downstream callers do not rely on the old signature.
.storybook/preview.ts (1)
14-14: LGTM! Minor formatting improvements.
The formatting changes align with the new ESLint configuration.
Also applies to: 25-25, 27-27
package.json (1)
Line range hint 161-166: LGTM! Improved lint-staged configuration.
The lint-staged configuration has been updated to include TypeScript and JSX files, which aligns well with the project's needs.
| attachReact(element?: any) { | ||
| // eslint-disable-next-line react/no-render-return-value | ||
| // eslint-disable-next-line react/no-render-return-value,react/no-deprecated |
There was a problem hiding this comment.
Avoid relying on the return value of ReactDOM.render.
As per React’s documentation, the return value from ReactDOM.render is considered legacy and may no longer be reliable in future versions of React.
- return ReactDOM.render(this.renderReact(), element);
+ ReactDOM.render(this.renderReact(), element);
+ return undefined;Committable suggestion skipped: line range outside the PR's diff.
| import { INITIAL_VIEWPORTS } from "@storybook/addon-viewport"; | ||
| import { Formio, Templates } from "@tsed/react-formio"; | ||
| import tailwind from "@tsed/tailwind-formio"; | ||
|
|
||
| // eslint-disable-next-line react-hooks/rules-of-hooks | ||
| Formio.use(tailwind); | ||
| Templates.framework = "tailwind"; |
There was a problem hiding this comment.
💡 Codebase verification
Remove unnecessary ESLint directive for hooks rule
The ESLint directive to disable react-hooks/rules-of-hooks appears to be unnecessary because:
Formio.use()is a regular method call, not a React Hook- There's no evidence of React Hook usage in the storybook configuration
- The same
Formio.use(tailwind)pattern is used in other files without requiring the ESLint directive
You can safely remove the // eslint-disable-next-line react-hooks/rules-of-hooks comment.
🔗 Analysis chain
Verify the necessity of disabling the hooks rule.
The ESLint directive to disable react-hooks/rules-of-hooks has been added. However, since Formio.use() is not a React Hook, this directive might be unnecessary. Consider removing it if the ESLint warning is no longer present with the new ESLint configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the hooks rule warning occurs without the disable directive
# First, create a temporary file without the eslint-disable comment
cat > temp.ts << 'EOL'
import { INITIAL_VIEWPORTS } from "@storybook/addon-viewport";
import { Formio, Templates } from "@tsed/react-formio";
import tailwind from "@tsed/tailwind-formio";
Formio.use(tailwind);
Templates.framework = "tailwind";
EOL
# Run ESLint on the temporary file
eslint temp.ts
Length of output: 120
Script:
#!/bin/bash
# Let's check if there are any similar ESLint directives in the codebase
# and understand the context better
rg "eslint-disable.*react-hooks" -A 2 -B 2
# Also check for any Formio.use patterns to see how they're used elsewhere
rg "Formio\.use" -A 2 -B 2
# Let's also check the ESLint configuration to understand the rules
fd -e .eslintrc -e .eslintrc.js -e .eslintrc.json -e .eslintrc.yml
Length of output: 564
Script:
#!/bin/bash
# Let's check the storybook configuration files for any React-related setup
# that might explain the hooks rule
fd -g "*.js" -g "*.ts" .storybook/
# Also check for any React hooks usage in the storybook files
rg "use[A-Z]" .storybook/ -A 2 -B 2
# Check if there are any other instances of Formio.use in storybook files
rg "Formio" .storybook/ -A 2 -B 2
Length of output: 636
| "lint": "eslint", | ||
| "lint:fix": "eslint", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Lint scripts might be too minimal.
The lint scripts have been simplified from using Lerna to direct ESLint calls, but they're missing necessary arguments:
- No file patterns specified
- No configuration file referenced
- No additional flags for fixing
Consider updating the scripts to be more explicit:
- "lint": "eslint",
- "lint:fix": "eslint",
+ "lint": "eslint . --ext .js,.jsx,.ts,.tsx",
+ "lint:fix": "eslint . --ext .js,.jsx,.ts,.tsx --fix"📝 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.
| "lint": "eslint", | |
| "lint:fix": "eslint", | |
| "lint": "eslint . --ext .js,.jsx,.ts,.tsx", | |
| "lint:fix": "eslint . --ext .js,.jsx,.ts,.tsx --fix" |
| "@typescript-eslint/eslint-plugin": "8.18.2", | ||
| "@typescript-eslint/parser": "8.18.2", | ||
| "@vitejs/plugin-react": "^4.3.4", | ||
| "autoprefixer": "^10.4.7", | ||
| "babel-eslint": "^10.1.0", | ||
| "camelcase": "6.3.0", | ||
| "chromatic": "11.20.2", | ||
| "cross-env": "7.0.3", | ||
| "eslint": "^8.15.0", | ||
| "eslint-config-prettier": "^8.5.0", | ||
| "eslint-config-react-app": "^7.0.1", | ||
| "eslint-plugin-import": "^2.26.0", | ||
| "eslint-plugin-jsx-a11y": "^6.5.1", | ||
| "eslint-plugin-prettier": "^4.2.1", | ||
| "eslint-plugin-react": "^7.30.1", | ||
| "eslint-plugin-react-hooks": "^4.6.0", | ||
| "eslint-plugin-simple-import-sort": "^7.0.0", | ||
| "eslint-plugin-testing-library": "^5.5.1", | ||
| "eslint-plugin-workspaces": "^0.7.0", | ||
| "eslint": "9.17.0", | ||
| "eslint-config-prettier": "9.1.0", | ||
| "eslint-plugin-jsx-a11y": "6.10.2", | ||
| "eslint-plugin-prettier": "5.2.1", | ||
| "eslint-plugin-react": "7.37.3", | ||
| "eslint-plugin-react-hooks": "5.1.0", | ||
| "eslint-plugin-simple-import-sort": "12.1.1", | ||
| "eslint-plugin-storybook": "0.11.1", | ||
| "eslint-plugin-testing-library": "7.1.1", | ||
| "eslint-plugin-workspaces": "0.10.1", |
There was a problem hiding this comment.
💡 Codebase verification
ESLint version needs to be downgraded to 8.57.0
The dependency check reveals an incompatibility in the current setup:
@typescript-eslint/[email protected]requires ESLint version^8.57.0 || ^9.0.0- Current ESLint version is
9.17.0 - Latest version of
@typescript-eslint/eslint-pluginis still 8.18.2
To resolve this, you should downgrade ESLint to version 8.57.0 since:
- The TypeScript ESLint plugin is at its latest version but requires ESLint 8.57.0
- Other plugins (react, react-hooks) are compatible with ESLint 8.x
- This ensures maximum compatibility across all plugins
🔗 Analysis chain
Verify ESLint plugin compatibility.
The ESLint upgrade to version 9.x comes with several plugin updates. The versions look compatible, but let's verify the peer dependencies.
ESLint plugin compatibility issue detected with @typescript-eslint/eslint-plugin
The peer dependency check reveals an incompatibility:
@typescript-eslint/[email protected]requireseslint: ^8.57.0 || ^9.0.0, but the project uses[email protected]- Other plugins (
eslint-plugin-reactandeslint-plugin-react-hooks) are compatible with ESLint 9.x
Let's verify if there's a newer version of @typescript-eslint/eslint-plugin that might be compatible:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check peer dependency compatibility
npm info [email protected] peerDependencies
npm info @typescript-eslint/[email protected] peerDependencies
npm info [email protected] peerDependencies
npm info [email protected] peerDependencies
Length of output: 503
Script:
#!/bin/bash
# Check latest version of @typescript-eslint/eslint-plugin
npm info @typescript-eslint/eslint-plugin dist-tags.latest
Length of output: 67
|
🎉 This PR is included in version 3.0.0-alpha.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.3.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
DefaultDndRowcomponent.DefaultOperationButtoncomponent for enhanced data handling.build-colors.jsscript for better visibility of errors.Bug Fixes
getAccessandgetProjectAccessfunctions to ensure better debugging.Chores
Tests