Repository navigation
Conversation
There was a problem hiding this comment.
5 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/antalmanac-planner/site/src/component/RecentOfferingsTooltip/RecentOfferingsTooltip.tsx">
<violation number="1" location="apps/antalmanac-planner/site/src/component/RecentOfferingsTooltip/RecentOfferingsTooltip.tsx:14">
P2: Do not snapshot `new Date()` for the component lifetime; this leaves the previous-academic-year badges stale across October 1. Recalculate the date when rendering, or add an explicit time-based refresh if a stable render value is required.</violation>
</file>
<file name="apps/antalmanac-planner/site/src/app/roadmap/planner/AddYearPopup.tsx">
<violation number="1" location="apps/antalmanac-planner/site/src/app/roadmap/planner/AddYearPopup.tsx:29">
P2: `currentDate` is captured only when `AddYearPopup` mounts, so an empty roadmap opened after New Year can default to the previous year. Refresh the date when opening the add-year modal (or otherwise derive the current year at that user action) instead of retaining this mount-time snapshot.</violation>
</file>
<file name="apps/antalmanac-planner/site/src/app/roadmap/planner/Quarter.tsx">
<violation number="1" location="apps/antalmanac-planner/site/src/app/roadmap/planner/Quarter.tsx:61">
P2: This freezes the planner's academic-year calculation for the lifetime of each mounted `Quarter`. If a planner remains open across the September boundary, course offering warnings continue using the previous academic year; refresh the date/year at the boundary or otherwise use a refreshable current-year source.</violation>
</file>
<file name="apps/antalmanac-scheduler/site/src/components/RightPane/CoursePane/CourseRenderPane/RecruitmentBanner.tsx">
<violation number="1" location="apps/antalmanac-scheduler/site/src/components/RightPane/CoursePane/CourseRenderPane/RecruitmentBanner.tsx:15">
P2: This mount-time `now` snapshot can prevent an expired dismissal from ever becoming eligible again while the course results pane stays mounted. Update the reference time when checking expiry (or schedule periodic refreshes) so the 11-week dismissal window is measured against the current time rather than the component mount time.</violation>
</file>
<file name="apps/antalmanac-planner/site/.oxlintrc.json">
<violation number="1" location="apps/antalmanac-planner/site/.oxlintrc.json:24">
P2: Enabling `react/purity` still leaves the planner `Logo` component constructing a `Date` during render, so the planner lint target will fail. Move this date into a lazy state or otherwise fix the render-time impure call before enabling the rule.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| if (terms.length === 0) return null; | ||
|
|
||
| const prevYear = new Date().getMonth() > 9 ? new Date().getFullYear() : new Date().getFullYear() - 1; | ||
| const [currentDate] = useState(() => new Date()); |
There was a problem hiding this comment.
P2: Do not snapshot new Date() for the component lifetime; this leaves the previous-academic-year badges stale across October 1. Recalculate the date when rendering, or add an explicit time-based refresh if a stable render value is required.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/antalmanac-planner/site/src/component/RecentOfferingsTooltip/RecentOfferingsTooltip.tsx, line 14:
<comment>Do not snapshot `new Date()` for the component lifetime; this leaves the previous-academic-year badges stale across October 1. Recalculate the date when rendering, or add an explicit time-based refresh if a stable render value is required.</comment>
<file context>
@@ -11,7 +11,9 @@ interface RecentOfferingsTooltipProps {
if (terms.length === 0) return null;
- const prevYear = new Date().getMonth() > 9 ? new Date().getFullYear() : new Date().getFullYear() - 1;
+ const [currentDate] = useState(() => new Date());
+
+ const prevYear = currentDate.getMonth() > 9 ? currentDate.getFullYear() : currentDate.getFullYear() - 1;
</file context>
| const currentPlan = useAppSelector(selectCurrentPlan); | ||
| const plannerYears = currentPlan.content.yearPlans; | ||
| const dispatch = useAppDispatch(); | ||
| const [currentDate] = useState(() => new Date()); |
There was a problem hiding this comment.
P2: currentDate is captured only when AddYearPopup mounts, so an empty roadmap opened after New Year can default to the previous year. Refresh the date when opening the add-year modal (or otherwise derive the current year at that user action) instead of retaining this mount-time snapshot.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/antalmanac-planner/site/src/app/roadmap/planner/AddYearPopup.tsx, line 29:
<comment>`currentDate` is captured only when `AddYearPopup` mounts, so an empty roadmap opened after New Year can default to the previous year. Refresh the date when opening the add-year modal (or otherwise derive the current year at that user action) instead of retaining this mount-time snapshot.</comment>
<file context>
@@ -26,10 +26,11 @@ const AddYearPopup: FC<AddYearProps> = ({ buttonSize }) => {
const currentPlan = useAppSelector(selectCurrentPlan);
const plannerYears = currentPlan.content.yearPlans;
const dispatch = useAppDispatch();
+ const [currentDate] = useState(() => new Date());
const placeholderName = 'Year ' + (plannerYears.length + 1);
</file context>
| const startYear = currentPlan.content.yearPlans[yearIndex].startYear; | ||
| const courses = data.courses; | ||
|
|
||
| const [currentDate] = useState(() => new Date()); |
There was a problem hiding this comment.
P2: This freezes the planner's academic-year calculation for the lifetime of each mounted Quarter. If a planner remains open across the September boundary, course offering warnings continue using the previous academic year; refresh the date/year at the boundary or otherwise use a refreshable current-year source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/antalmanac-planner/site/src/app/roadmap/planner/Quarter.tsx, line 61:
<comment>This freezes the planner's academic-year calculation for the lifetime of each mounted `Quarter`. If a planner remains open across the September boundary, course offering warnings continue using the previous academic year; refresh the date/year at the boundary or otherwise use a refreshable current-year source.</comment>
<file context>
@@ -58,6 +58,8 @@ const Quarter: FC<QuarterProps> = ({ yearIndex, quarterIndex, data }) => {
const startYear = currentPlan.content.yearPlans[yearIndex].startYear;
const courses = data.courses;
+ const [currentDate] = useState(() => new Date());
+
// Calculate Quarter Stats
</file context>
|
|
||
| export function RecruitmentBanner({ deptValue }: RecruitmentBannerProps) { | ||
| const [bannerVisibility, setBannerVisibility] = useState(true); | ||
| const [now, setNow] = useState(() => Date.now()); |
There was a problem hiding this comment.
P2: This mount-time now snapshot can prevent an expired dismissal from ever becoming eligible again while the course results pane stays mounted. Update the reference time when checking expiry (or schedule periodic refreshes) so the 11-week dismissal window is measured against the current time rather than the component mount time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/antalmanac-scheduler/site/src/components/RightPane/CoursePane/CourseRenderPane/RecruitmentBanner.tsx, line 15:
<comment>This mount-time `now` snapshot can prevent an expired dismissal from ever becoming eligible again while the course results pane stays mounted. Update the reference time when checking expiry (or schedule periodic refreshes) so the 11-week dismissal window is measured against the current time rather than the component mount time.</comment>
<file context>
@@ -12,17 +12,19 @@ interface RecruitmentBannerProps {
export function RecruitmentBanner({ deptValue }: RecruitmentBannerProps) {
const [bannerVisibility, setBannerVisibility] = useState(true);
+ const [now, setNow] = useState(() => Date.now());
+
const theme = useTheme();
</file context>
| "react/purity": "off", | ||
| "react/preserve-manual-memoization": "off", | ||
| "react/refs": "off" | ||
| "react/set-state-in-effect": "off" |
There was a problem hiding this comment.
P2: Enabling react/purity still leaves the planner Logo component constructing a Date during render, so the planner lint target will fail. Move this date into a lazy state or otherwise fix the render-time impure call before enabling the rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/antalmanac-planner/site/.oxlintrc.json, line 24:
<comment>Enabling `react/purity` still leaves the planner `Logo` component constructing a `Date` during render, so the planner lint target will fail. Move this date into a lazy state or otherwise fix the render-time impure call before enabling the rule.</comment>
<file context>
@@ -21,10 +21,7 @@
- "react/purity": "off",
- "react/preserve-manual-memoization": "off",
- "react/refs": "off"
+ "react/set-state-in-effect": "off"
},
"settings": {
</file context>
Summary
Doesn't fix
react/set-state-in-effectbecause those are pretty complicated and probably need their own followup PR.Screenshots
Test Plan
react/set-state-in-effectIssues
Closes #