Migrate to BlueprintJS v6 #3289
Conversation
|
@RichDom2185 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: RichDom2185 <[email protected]>
…anges Co-authored-by: RichDom2185 <[email protected]>
…functionality Co-authored-by: RichDom2185 <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughThis pull request upgrades the project to BlueprintJS v6, updating relevant dependencies, component imports, and prop usages to match breaking changes. It replaces deprecated BlueprintJS packages and components, updates SCSS namespace variables, and modifies usage of props such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReactComponent
participant BlueprintJS_v6
User->>ReactComponent: Interacts with UI (e.g., clicks, edits)
ReactComponent->>BlueprintJS_v6: Uses updated components/props (e.g., Icon size, DateInput)
BlueprintJS_v6-->>ReactComponent: Renders updated UI elements
ReactComponent-->>User: Displays UI with BlueprintJS v6 changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
🔭 Outside diff range comments (2)
eslint.config.mjs (1)
51-53: Remove deprecated/removed TypeScript-ESLint rules
@typescript-eslint/interface-name-prefixand@typescript-eslint/camelcaseare long-removed and will cause “Definition for rule … was not found” errors. Drop them.Apply:
- '@typescript-eslint/interface-name-prefix': 'off', - '@typescript-eslint/camelcase': 'off',src/pages/academy/groundControl/subcomponents/GroundControlEditCell.tsx (1)
54-57: Fix null handling: on clearing the DateInput,dayjs(null)sets “now” instead of preserving nullCurrently, when the user clears the input,
onChangereceivesnull, butdayjs(null)produces the current time. This makesnewDatenevernull, so the “No date and time selected!” branch will not trigger, and you may inadvertently save “now”.
- Preserve
nullin state whenselectedDateisnull.- Also pass
null(notundefined) to the controlledvalueprop when there’s no date.- const handleDateChange = React.useCallback( - (selectedDate: string | null) => setNewDate(dayjs(selectedDate)), - [] - ); + const handleDateChange = React.useCallback( + (selectedDate: string | null) => setNewDate(selectedDate ? dayjs(selectedDate) : null), + [] + ); @@ - value={newDate?.toISOString()} + value={newDate ? newDate.toISOString() : null}Also applies to: 65-79
🧹 Nitpick comments (5)
.prettierrc (1)
6-7: End-of-line handling: consider normalizing via .gitattributes
endOfLine: "auto"can lead to cross-OS line-ending churn. Either set"lf"here or add a.gitattributesto normalize line endings repo-wide.Suggested
.gitattributes:* text=auto eol=lfsrc/commons/sideContent/content/githubAssessments/SideContentMarkdownEditor.tsx (1)
41-41: Use controlled input to avoid uncontrolled/controlled driftSince you update
contenton change, prefervalue={content}overdefaultValuefor consistency and to prevent stale UI whencontentchanges externally.Apply:
- <TextArea onChange={onEditorChange} fill={true} autoResize={true} defaultValue={content} /> + <TextArea onChange={onEditorChange} fill={true} autoResize={true} value={content} />src/commons/mobileWorkspace/mobileSideContent/MobileSideContent.tsx (1)
40-40: Use consistent constant naming to avoid confusion with removed prop nameProp rename to size is correct for Blueprint v6. Minor nit: the local constant is still named iconSize, which can be misleading now that the prop is size. Rename for clarity.
Apply:
- const iconSize = 20; + const ICON_SIZE = 20; ... - <Icon icon={tab.iconName} size={iconSize} /> + <Icon icon={tab.iconName} size={ICON_SIZE} />Also applies to: 27-27
src/commons/achievement/control/achievementEditor/EditableDate.tsx (1)
13-15: Nit: Avoid stale state in toggle by using functional updaterUse the functional form to prevent potential stale closures if multiple toggles happen quickly.
- const toggleOpen = () => setOpen(!isOpen); + const toggleOpen = () => setOpen(prev => !prev);src/pages/academy/groundControl/subcomponents/GroundControlEditCell.tsx (1)
20-21: Nit: Off-by-one day in minDate
new Date(2010, 0, 0)resolves to 2009-12-31. If you intend Jan 1, 2010, use day 1.- const minDate = new Date(2010, 0, 0); + const minDate = new Date(2010, 0, 1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (22)
src/commons/__tests__/__snapshots__/ContentDisplay.test.tsx.snapis excluded by!**/*.snapsrc/commons/__tests__/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snapsrc/commons/assessment/__tests__/__snapshots__/Assessment.test.tsx.snapis excluded by!**/*.snapsrc/commons/assessmentWorkspace/__tests__/__snapshots__/AssessmentWorkspace.test.tsx.snapis excluded by!**/*.snapsrc/commons/dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snapis excluded by!**/*.snapsrc/commons/navigationBar/__tests__/__snapshots__/NavigationBar.test.tsx.snapis excluded by!**/*.snapsrc/commons/navigationBar/subcomponents/__tests__/__snapshots__/AcademyNavigationBar.test.tsx.snapis excluded by!**/*.snapsrc/commons/navigationBar/subcomponents/__tests__/__snapshots__/NavigationBarLangSelectButton.test.tsx.snapis excluded by!**/*.snapsrc/commons/navigationBar/subcomponents/__tests__/__snapshots__/SicpNavigationBar.test.tsx.snapis excluded by!**/*.snapsrc/commons/repl/__tests__/__snapshots__/Repl.test.tsx.snapis excluded by!**/*.snapsrc/commons/sideContent/__tests__/__snapshots__/SideContentAutograder.test.tsx.snapis excluded by!**/*.snapsrc/commons/sideContent/__tests__/__snapshots__/SideContentContestLeaderboard.test.tsx.snapis excluded by!**/*.snapsrc/commons/sideContent/__tests__/__snapshots__/SideContentContestVoting.test.tsx.snapis excluded by!**/*.snapsrc/commons/sideContent/__tests__/__snapshots__/SideContentCseMachine.test.tsx.snapis excluded by!**/*.snapsrc/features/sicp/errors/__tests__/__snapshots__/SicpErrors.test.tsx.snapis excluded by!**/*.snapsrc/features/sicp/parser/__tests__/__snapshots__/ParseJson.test.tsx.snapis excluded by!**/*.snapsrc/pages/playground/__tests__/__snapshots__/Playground.test.tsx.snapis excluded by!**/*.snapsrc/pages/sicp/subcomponents/__tests__/__snapshots__/CodeSnippet.test.tsx.snapis excluded by!**/*.snapsrc/pages/sicp/subcomponents/__tests__/__snapshots__/SicpExercise.test.tsx.snapis excluded by!**/*.snapsrc/pages/sicp/subcomponents/__tests__/__snapshots__/SicpIndexPage.test.tsx.snapis excluded by!**/*.snapsrc/pages/sicp/subcomponents/__tests__/__snapshots__/SicpToc.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
.prettierrc(1 hunks)eslint.config.mjs(1 hunks)package.json(1 hunks)src/commons/achievement/AchievementFilter.tsx(1 hunks)src/commons/achievement/AchievementView.tsx(1 hunks)src/commons/achievement/control/achievementEditor/EditableDate.tsx(1 hunks)src/commons/achievement/control/goalEditor/EditableDate.tsx(1 hunks)src/commons/assessment/Assessment.tsx(1 hunks)src/commons/editingOverviewCard/EditingOverviewCard.tsx(1 hunks)src/commons/mobileWorkspace/mobileSideContent/MobileSideContent.tsx(1 hunks)src/commons/sideContent/content/githubAssessments/SideContentMarkdownEditor.tsx(1 hunks)src/pages/academy/groundControl/subcomponents/GroundControlEditCell.tsx(2 hunks)src/styles/_global.scss(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/commons/assessment/Assessment.tsx (1)
src/commons/utils/DateHelper.ts (2)
beforeNow(16-20)getPrettyDate(32-36)
🔇 Additional comments (11)
package.json (3)
34-37: PR summary inconsistency: dependencies claimed removed still presentPR summary mentions removing problematic deps (
conductor,language-directory,xlsx), but they are still listed (Lines 56, 70, 110). Please confirm intent and either remove them or adjust the summary.If removal is intended, also search for usages to ensure stubs cover them:
#!/bin/bash rg -n --hidden --glob '!node_modules' \ "from ['\"](conductor|language-directory|xlsx)['\"]|require\\(['\"](conductor|language-directory|xlsx)['\"]\\)" -SLikely an incorrect or invalid review comment.
44-44: Pinned version change — confirm rationale
@sourceacademy/sharedb-aceis pinned to2.1.1(previously a range per summary). If the pin is to avoid install breaks, consider adding a comment in package.json or a Renovate/Dependabot rule to revisit later.
34-37: No deprecated@blueprintjs/datetime2imports remainVerified that there are no occurrences of
@blueprintjs/datetime2in the codebase. The Blueprint v6 dependency upgrade is complete.eslint.config.mjs (1)
7-7: Import position change is fineNo functional impact; import order here is non-semantic in flat config.
src/commons/sideContent/content/githubAssessments/SideContentMarkdownEditor.tsx (1)
41-41: Correct prop migration:growVertically→autoResizeThe v6 prop change is applied correctly.
src/commons/achievement/AchievementView.tsx (1)
62-62: LGTM: Icon prop migrationUsing size={60} is correct for Blueprint v6. No issues spotted.
src/commons/achievement/AchievementFilter.tsx (1)
22-22: LGTM: Icon prop migrationProp rename to size={30} is correct. Looks good.
src/commons/editingOverviewCard/EditingOverviewCard.tsx (1)
116-116: LGTM: Icon prop migrationProp rename to size={12} is correct for Blueprint v6.
src/commons/achievement/control/achievementEditor/EditableDate.tsx (1)
35-39: LGTM: DatePicker v6 onChange null handling is correctMapping
nulltoundefinedkeepschangeDate’s(date?: Date)contract intact and avoids passing falsy values through.src/commons/achievement/control/goalEditor/EditableDate.tsx (1)
35-39: LGTM: DatePicker v6 onChange null handling is correctConsistently mapping
nulltoundefinedaligns with the updated API and yourchangeDatesignature.src/pages/academy/groundControl/subcomponents/GroundControlEditCell.tsx (1)
2-2: No residual @blueprintjs/datetime2 or v3 component references detectedI ran the provided grep scans for
@blueprintjs/datetime2,DateInput3,DatePicker3,RangeInput3,TimezoneSelect, andTimezonePicker—no matches were found. The migration to Blueprint v6’sDateInputappears complete.
This PR migrates the frontend from Blueprint v5 to v6, implementing all required breaking changes as outlined in the Blueprint v6 migration guide.
Changes Made
Dependencies Updated
@blueprintjs/core: ^5.10.1 → ^6.0.0@blueprintjs/select: ^5.1.3 → ^6.0.0@blueprintjs/datetime: Added ^6.0.0 (replaces datetime2)@blueprintjs/datetime2: Removed (deprecated in v6)@blueprintjs/icons: Already on ^6.0.0Breaking Changes Fixed
CSS Namespace
bp5-tobp6-in SCSS variablesIcon Component
iconSizeprop withsizeprop across all Icon usagesDatePicker Components
@blueprintjs/datetime2to@blueprintjs/datetimeDateInput3→DateInputimportsonChangecallback signature changes:(date?: Date) => void→(selectedDate: Date | null, isUserChange: boolean) => voidTextArea Component
growVerticallyprop withautoResizepropExternal Dependencies Removed
conductor,language-directory, andxlsxdependencies as they were causing installation failuresTesting
Remaining Issues
8 pre-existing TypeScript errors remain that are unrelated to Blueprint migration:
These errors existed before the migration and are not caused by Blueprint v6 changes.
The migration successfully updates all Blueprint ecosystem dependencies and applies the necessary breaking changes, ensuring compatibility with Blueprint v6 while maintaining existing functionality.
Fixes #3288.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
cdn.sheetjs.comnode /usr/local/bin/yarn install(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores