Upgrade to React 18 #621
Upgrade to React 18 #621
Conversation
… newer react. Remove one very simple test that depended on it.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request encompasses a series of updates across the admin section of the project, focusing on modernizing React components, updating library imports, and refining build configurations. The changes primarily involve migrating to newer React rendering methods, updating import statements, and adjusting dependency specifications. The modifications touch multiple files, including Jest setup, component implementations, and webpack configuration. Changes
Possibly related PRs
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 review |
✅ Actions performedReview triggered.
|
![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 (1)
admin/src/Components/DatePeriodInput.tsx (1)
139-147: Consider using a reducer for state management.The date handling logic could be simplified using a reducer pattern, especially since you're managing related state (start/end dates).
+type Action = + | { type: 'SET_START_DATE'; date: Date | null } + | { type: 'SET_END_DATE'; date: Date | null }; + +function dateReducer(state: DateState, action: Action) { + switch (action.type) { + case 'SET_START_DATE': + return { ...state, start: action.date }; + case 'SET_END_DATE': + return { ...state, end: action.date }; + } +}🧰 Tools
🪛 GitHub Actions: Check formatting of source code
[warning] Code formatting does not match Prettier standards
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
admin/dist/css/default.cssis excluded by!**/dist/**admin/package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonadmin/package.jsonis excluded by!**/*.json
📒 Files selected for processing (10)
admin/jestSetup.js(1 hunks)admin/src/Components/DatePeriodInput.js(0 hunks)admin/src/Components/DatePeriodInput.tsx(1 hunks)admin/src/Components/MessageForm.jsx(1 hunks)admin/src/Components/__test__/404.test.js(0 hunks)admin/src/Membership/GroupBoxMembers.jsx(1 hunks)admin/src/Membership/MemberBoxSpans.jsx(1 hunks)admin/src/app.jsx(1 hunks)admin/webpack.config.js(2 hunks)requirements.txt(1 hunks)
💤 Files with no reviewable changes (2)
- admin/src/Components/DatePeriodInput.js
- admin/src/Components/test/404.test.js
✅ Files skipped from review due to trivial changes (4)
- admin/src/Membership/MemberBoxSpans.jsx
- admin/src/Components/MessageForm.jsx
- admin/jestSetup.js
- admin/src/Membership/GroupBoxMembers.jsx
🧰 Additional context used
🪛 GitHub Actions: Check formatting of source code
admin/webpack.config.js
[warning] Code formatting does not match Prettier standards
admin/src/Components/DatePeriodInput.tsx
[warning] Code formatting does not match Prettier standards
🔇 Additional comments (6)
admin/webpack.config.js (2)
73-73: LGTM! Fixed webpack extension warning.Changed from
"*"to".*"to ensure all extensions begin with a dot, following webpack's best practices.🧰 Tools
🪛 GitHub Actions: Check formatting of source code
[warning] Code formatting does not match Prettier standards
12-13: Verify shell command input sanitization.The concatenated path should be properly escaped to prevent potential shell injection.
✅ Verification successful
Shell command is properly sanitized
The
srcpath is safely constructed usingpath.resolve()with trusted inputs (__dirname), which automatically handles path normalization and special characters. The string concatenation change doesn't introduce any security concerns.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the src variable contains any special characters that could be used for command injection echo "Checking src path for special characters:" fd . --type d --max-depth 1 --exec sh -c 'echo "Directory: {}"; echo "{}" | grep -P "[\$&;`\\|]"'Length of output: 498
Script:
#!/bin/bash # Examine the webpack config to understand src variable usage echo "Checking webpack config for src variable definition and usage:" rg -A 5 -B 5 "src +" admin/webpack.config.jsLength of output: 647
🧰 Tools
🪛 GitHub Actions: Check formatting of source code
[warning] Code formatting does not match Prettier standards
admin/src/Components/DatePeriodInput.tsx (2)
53-66: Consider handling edge cases in date selection.The commented-out code for handling undefined dates suggests there might be edge cases that need attention. Consider implementing proper error handling for these cases.
🧰 Tools
🪛 GitHub Actions: Check formatting of source code
[warning] Code formatting does not match Prettier standards
81-102: Great accessibility implementation!Excellent use of ARIA attributes and semantic HTML. The calendar button is properly labeled and connected to its dialog.
🧰 Tools
🪛 GitHub Actions: Check formatting of source code
[warning] Code formatting does not match Prettier standards
admin/src/app.jsx (1)
225-225: LGTM! Proper React 18 root API implementation.Correctly migrated from
ReactDOM.rendertocreateRootAPI for React 18 concurrent features.requirements.txt (1)
3-3: LGTM! Pinned ruff version for CI consistency.Version pinning will ensure consistent linting behavior across environments.
emanuelen5
left a comment
emanuelen5
left a comment
There was a problem hiding this comment.
Awesome! Build errors must be resolved, but the changes are great!
|
You have a failing lint test: https://github.com/makerspace/makeradmin/actions/runs/12787676370/job/35647356284 |
|
Yeah. Fixed in the typescript PR. |
Type checking and formatting may be off in this PR. I have another one coming for typescript stuff.