-
Notifications
You must be signed in to change notification settings - Fork 5
Land dependency modernization stack: security bumps, TS6+Vite8, lazy catalogs, MUI 9, dep-shedding #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Land dependency modernization stack: security bumps, TS6+Vite8, lazy catalogs, MUI 9, dep-shedding #166
Changes from all commits
adfff96
95088ab
d0394da
06f49ee
00a6cc3
b88d8f0
21339b8
8e8bdfb
ecff48c
0fc9466
9e6810e
e6c7bf1
83e84cf
39cfb1b
56fba89
2daf2d9
97a90fe
c24b0d9
08bdb19
d6f4411
ffa1806
015eb5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
|
devin-ai-integration[bot] marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,5 @@ | ||
| import { use, useState } from "react"; | ||
| import Modal, { useModal } from "./Modal"; | ||
| import Select from "react-select"; | ||
| import { SingleValue } from "react-select"; | ||
| import { DragDropContext, Droppable, Draggable } from "@hello-pangea/dnd"; | ||
| import { Fzf } from "fzf"; | ||
| import { createEvents } from "ics"; | ||
|
|
@@ -22,7 +20,13 @@ import { | |
| decompressFromEncodedURIComponent, | ||
| } from "lz-string"; | ||
|
|
||
| import { Collapse, IconButton, Switch } from "@mui/material"; | ||
| import { | ||
| Autocomplete, | ||
| Collapse, | ||
| IconButton, | ||
| Switch, | ||
| TextField, | ||
| } from "@mui/material"; | ||
| import { UnfoldLess, UnfoldMore } from "@mui/icons-material"; | ||
| import { useAutoAnimate } from "@formkit/auto-animate/react"; | ||
| import TERM_START_DATES from "./data/term_start_dates.json"; | ||
|
|
@@ -193,9 +197,9 @@ function SectionDropdown(props: { course: CourseStorage }) { | |
| const course = props.course; | ||
| const state = use(AppState); | ||
|
|
||
| const onChange = (newSection: SingleValue<Maybe<SectionData>>) => { | ||
| const onChange = (newSection: Maybe<SectionData>) => { | ||
| course.sectionId = | ||
| newSection !== null | ||
| newSection != null | ||
| ? course.courseData.sections.findIndex( | ||
| (s) => s.number === newSection.number, | ||
| ) | ||
|
Comment on lines
+200
to
205
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: SectionDropdown.onChange directly mutates course.sectionId before calling addCourse At (Refers to lines 200-208) Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — pre-existing mutation pattern, not introduced by this PR. As noted, |
||
|
|
@@ -206,27 +210,24 @@ function SectionDropdown(props: { course: CourseStorage }) { | |
|
|
||
| return ( | ||
| <div className="workspace-entry-section"> | ||
| <Select | ||
| isClearable | ||
| placeholder="" | ||
| <Autocomplete | ||
| size="small" | ||
| value={ | ||
| course.sectionId !== null | ||
| ? course.courseData.sections.find( | ||
| ? (course.courseData.sections.find( | ||
| (c) => | ||
| c.number === | ||
| course.courseData.sections[course.sectionId!].number, | ||
| ) | ||
| ) ?? null) | ||
| : null | ||
|
Comment on lines
215
to
222
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: SectionDropdown value lookup is redundant but necessary for MUI Autocomplete referential identity The Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
| onChange={onChange} | ||
| onChange={(_event, newSection) => onChange(newSection)} | ||
| options={course.courseData.sections} | ||
| getOptionLabel={(section) => `${section.number}`} | ||
| isOptionSelected={(section) => | ||
| course.sectionId !== null | ||
| ? section.number === | ||
| course.courseData.sections[course.sectionId].number | ||
| : false | ||
| isOptionEqualToValue={(option, selected) => | ||
| option.number === selected.number | ||
| } | ||
| renderInput={(params) => <TextField {...params} aria-label="Section" />} | ||
| /> | ||
| </div> | ||
| ); | ||
|
|
@@ -409,20 +410,10 @@ function WorkspaceSearch() { | |
| const indexedCourses = use(AllCourses); | ||
| const courses = Object.values(indexedCourses); | ||
|
|
||
| // For some reason, options = [] on the second render, even though | ||
| // courses = [...] by then and options should equal courses. | ||
| // I came up with this hacky solution to get around that... | ||
| // The dropdown options should re-render properly | ||
| let [options, setOptions] = useState<CourseData[]>(courses); | ||
| const [firstLoad, setFirstLoad] = useState(true); | ||
| if (firstLoad && options.length === 0) { | ||
| options = courses; | ||
| } | ||
|
|
||
| const [selectedCourse, setCourse] = useState<Maybe<CourseData>>(null); | ||
|
|
||
| const handleSelect = (courseData: SingleValue<CourseData>) => { | ||
| setCourse(courseData as CourseData); | ||
| const handleSelect = (courseData: Maybe<CourseData>) => { | ||
| setCourse(courseData); | ||
| if (courseData) { | ||
| state.addCourse({ | ||
| courseData: courseData, | ||
|
|
@@ -438,25 +429,26 @@ function WorkspaceSearch() { | |
| selector: (item) => `${item.number} ${item.name}`, | ||
| }); | ||
|
Comment on lines
429
to
430
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Fzf instance recreated on every render in WorkspaceSearch The (Refers to lines 428-430) Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — correct that |
||
|
|
||
| const sortCourses = (input: string) => { | ||
| setOptions(fzf.find(input).map((item) => item.item)); | ||
| setFirstLoad(false); | ||
| }; | ||
|
|
||
| return ( | ||
| <Select | ||
| isClearable | ||
| <Autocomplete | ||
| size="small" | ||
| className="my-3" | ||
| placeholder="Add a course..." | ||
| options={options} | ||
| options={courses} | ||
| value={selectedCourse} | ||
| getOptionLabel={(course) => `${course?.number} - ${course?.name}`} | ||
| onChange={handleSelect} | ||
| isOptionSelected={(course) => course.id === selectedCourse?.id} | ||
| onInputChange={sortCourses} | ||
| filterOption={() => { | ||
| return true; | ||
| }} | ||
| getOptionLabel={(course) => `${course.number} - ${course.name}`} | ||
| isOptionEqualToValue={(option, selected) => option.id === selected.id} | ||
| onChange={(_event, courseData) => handleSelect(courseData)} | ||
| filterOptions={(allOptions, { inputValue }) => | ||
| inputValue ? fzf.find(inputValue).map((item) => item.item) : allOptions | ||
| } | ||
| renderInput={(params) => ( | ||
| <TextField | ||
| {...params} | ||
| placeholder={ | ||
| courses.length === 0 ? "Loading courses..." : "Add a course..." | ||
| } | ||
| /> | ||
| )} | ||
| /> | ||
| ); | ||
| } | ||
|
|
@@ -603,6 +595,10 @@ export default function Workspace({ term }: { term: string }) { | |
| }); | ||
|
|
||
| const importWorkspace = () => { | ||
| if (Object.keys(indexedCourses).length === 0) { | ||
| alert("Course data is still loading. Please try again in a moment."); | ||
| return; | ||
| } | ||
| const code = prompt("Copy in the workspace code.") || ""; | ||
| if (code === "") { | ||
| return; | ||
|
|
@@ -697,6 +693,12 @@ export default function Workspace({ term }: { term: string }) { | |
| <ControlButton | ||
| text="Default Schedule" | ||
| onClick={() => { | ||
| if (Object.keys(indexedCourses).length === 0) { | ||
| alert( | ||
| "Course data is still loading. Please try again in a moment.", | ||
| ); | ||
| return; | ||
| } | ||
| state.setCourses( | ||
| // Change based on term | ||
| (DEFAULT_COURSES[term.substring(0, 2)] ?? []).flatMap((name) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,39 @@ | ||
| import DATA_FA2023 from "./data/IndexedTotalFA2022-23.json"; | ||
| import DATA_WI2023 from "./data/IndexedTotalWI2022-23.json"; | ||
| import DATA_SP2023 from "./data/IndexedTotalSP2022-23.json"; | ||
| import DATA_FA2024 from "./data/IndexedTotalFA2023-24.json"; | ||
| import DATA_WI2024 from "./data/IndexedTotalWI2023-24.json"; | ||
| import DATA_SP2024 from "./data/IndexedTotalSP2023-24.json"; | ||
| import DATA_FA2025 from "./data/IndexedTotalFA2024-25.json"; | ||
| import DATA_WI2025 from "./data/IndexedTotalWI2024-25.json"; | ||
| import DATA_SP2025 from "./data/IndexedTotalSP2024-25.json"; | ||
| import DATA_FA2026 from "./data/IndexedTotalFA2025-26.json"; | ||
| import DATA_WI2026 from "./data/IndexedTotalWI2025-26.json"; | ||
| import DATA_SP2026 from "./data/IndexedTotalSP2025-26.json"; | ||
| import DATA_FA2027 from "./data/IndexedTotalFA2026-27.json"; | ||
|
|
||
| export const CURRENT_TERM = "/fa2027"; | ||
|
|
||
| export const courseDataSources: { | ||
| [key: string]: { [key: string]: CourseData }; | ||
| const courseDataLoaders: { | ||
| [key: string]: () => Promise<{ default: CourseIndex }>; | ||
| } = { | ||
| "/fa2023": DATA_FA2023, | ||
| "/wi2023": DATA_WI2023, | ||
| "/sp2023": DATA_SP2023, | ||
| "/fa2024": DATA_FA2024, | ||
| "/wi2024": DATA_WI2024, | ||
| "/sp2024": DATA_SP2024, | ||
| "/fa2025": DATA_FA2025, | ||
| "/wi2025": DATA_WI2025, | ||
| "/sp2025": DATA_SP2025, | ||
| "/fa2026": DATA_FA2026, | ||
| "/wi2026": DATA_WI2026, | ||
| "/sp2026": DATA_SP2026, | ||
| "/fa2027": DATA_FA2027, | ||
| "/fa2023": () => import("./data/IndexedTotalFA2022-23.json"), | ||
| "/wi2023": () => import("./data/IndexedTotalWI2022-23.json"), | ||
| "/sp2023": () => import("./data/IndexedTotalSP2022-23.json"), | ||
| "/fa2024": () => import("./data/IndexedTotalFA2023-24.json"), | ||
| "/wi2024": () => import("./data/IndexedTotalWI2023-24.json"), | ||
| "/sp2024": () => import("./data/IndexedTotalSP2023-24.json"), | ||
| "/fa2025": () => import("./data/IndexedTotalFA2024-25.json"), | ||
| "/wi2025": () => import("./data/IndexedTotalWI2024-25.json"), | ||
| "/sp2025": () => import("./data/IndexedTotalSP2024-25.json"), | ||
| "/fa2026": () => import("./data/IndexedTotalFA2025-26.json"), | ||
| "/wi2026": () => import("./data/IndexedTotalWI2025-26.json"), | ||
| "/sp2026": () => import("./data/IndexedTotalSP2025-26.json"), | ||
| "/fa2027": () => import("./data/IndexedTotalFA2026-27.json"), | ||
| }; | ||
|
|
||
| const courseIndexCache: { [key: string]: CourseIndex } = {}; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: Module-level courseIndexCache is a process-global singleton The Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| export function getCachedCourseIndex(term: string): CourseIndex | undefined { | ||
| return courseIndexCache[term]; | ||
| } | ||
|
|
||
| export async function loadCourseIndex(term: string): Promise<CourseIndex> { | ||
| const cached = courseIndexCache[term]; | ||
| if (cached) { | ||
| return cached; | ||
| } | ||
| const loader = courseDataLoaders[term]; | ||
| if (!loader) { | ||
| return {}; | ||
|
Comment on lines
+33
to
+34
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: loadCourseIndex does not cache empty results for unknown terms When Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
| const module = await loader(); | ||
| courseIndexCache[term] = module.default; | ||
| return module.default; | ||
|
Comment on lines
+27
to
+38
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: loadCourseIndex has benign duplicate-call race but no deduplication In Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — the duplicate-call race is benign (same module record resolves for both callers; dynamic |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| declare module "*.css"; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: New css.d.ts declaration covers all CSS imports needed by the project The new Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — informational only. The |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚩 Wrangler version pinned without caret, unlike other dependencies
In
package.json,wranglerchanged from^4.49.1(allows minor/patch updates) to4.98.0(exact version). All other dependencies use^ranges. This inconsistency might be intentional (to pin a known-good version) but could lead to the project falling behind on wrangler patches. Worth confirming this was deliberate.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deliberate: wrangler 4.99.0 (latest at time of writing) has a regression where
wrangler devreturns 404 for all static assets (bisected — 4.98.0 and earlier work). The exact pin prevents^from pulling in the broken version; it can be relaxed back to^once a fixed wrangler release ships.