-
Notifications
You must be signed in to change notification settings - Fork 0
β‘ Bolt: Optimize objective state updates #335
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
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,9 +118,10 @@ const TreatmentPlanManager: FC = () => { | |
| const [error, setError] = useState<string | null>(null) | ||
|
|
||
| const [isCreateModalOpen, setIsCreateModalOpen] = useState(false) | ||
| const [newPlanData, setNewPlanData] = useState<FormNewPlanData>( | ||
| JSON.parse(JSON.stringify(initialNewPlanData) as unknown), | ||
| ) | ||
| const [newPlanData, setNewPlanData] = useState<FormNewPlanData>({ | ||
| ...initialNewPlanData, | ||
| goals: [...initialNewPlanData.goals], | ||
| } as FormNewPlanData) | ||
|
|
||
| const [planToDelete, setPlanToDelete] = useState<TreatmentPlan | null>(null) | ||
| const [isEditModalOpen, setIsEditModalOpen] = useState(false) | ||
|
|
@@ -257,6 +258,7 @@ const TreatmentPlanManager: FC = () => { | |
| // --- End Goal Management --- | ||
|
|
||
| // --- Objective Management Functions --- | ||
| // β‘ Bolt: Replace JSON.parse/stringify with shallow copy for performance | ||
| const addObjective = (goalIndex: number, isEdit = false) => { | ||
| const newObjective: ClientSideNewObjective = { | ||
| description: '', | ||
|
|
@@ -265,27 +267,29 @@ const TreatmentPlanManager: FC = () => { | |
| } | ||
|
|
||
| if (isEdit && editingPlanData) { | ||
| const updatedGoals = JSON.parse( | ||
| JSON.stringify(editingPlanData.goals || []), | ||
| ) as EditableGoal[] | ||
| const updatedGoals = [...(editingPlanData.goals || [])] | ||
| if (updatedGoals[goalIndex]) { | ||
| updatedGoals[goalIndex].objectives = [ | ||
| ...(updatedGoals[goalIndex].objectives || []), | ||
| newObjective, | ||
| ] | ||
| updatedGoals[goalIndex] = { | ||
| ...updatedGoals[goalIndex], | ||
| objectives: [ | ||
| ...(updatedGoals[goalIndex].objectives || []), | ||
| newObjective, | ||
| ], | ||
| } | ||
| setEditingPlanData((prev: FormUpdatePlanData | null) => | ||
| prev ? { ...prev, goals: updatedGoals } : null, | ||
| ) | ||
| } | ||
| } else { | ||
| const updatedGoals = JSON.parse( | ||
| JSON.stringify(newPlanData.goals), | ||
| ) as ClientSideNewGoal[] | ||
| const updatedGoals = [...newPlanData.goals] | ||
| if (updatedGoals[goalIndex]) { | ||
| updatedGoals[goalIndex].objectives = [ | ||
| ...updatedGoals[goalIndex].objectives, | ||
| newObjective, | ||
| ] | ||
| updatedGoals[goalIndex] = { | ||
| ...updatedGoals[goalIndex], | ||
| objectives: [ | ||
| ...updatedGoals[goalIndex].objectives, | ||
| newObjective, | ||
| ], | ||
| } | ||
| setNewPlanData((prev: FormNewPlanData) => ({ | ||
| ...prev, | ||
| goals: updatedGoals, | ||
|
|
@@ -302,33 +306,41 @@ const TreatmentPlanManager: FC = () => { | |
| isEdit = false, | ||
| ) => { | ||
| if (isEdit && editingPlanData) { | ||
| const updatedGoals = JSON.parse( | ||
| JSON.stringify(editingPlanData.goals || []), | ||
| ) as EditableGoal[] | ||
| const updatedGoals = [...(editingPlanData.goals || [])] | ||
| if ( | ||
| updatedGoals[goalIndex] && | ||
| updatedGoals[goalIndex].objectives && | ||
| updatedGoals[goalIndex].objectives[objIndex] | ||
|
Comment on lines
310
to
313
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. suggestion (bug_risk): Guarding on In Suggested implementation: isEdit = false,
) => {
if (isEdit && editingPlanData) {
const updatedGoals = (editingPlanData.goals || []).map((goal) => ({
...goal,
objectives: goal.objectives || [],
}))
if (updatedGoals[goalIndex]?.objectives[objIndex]) {
const updatedObjectives = [...updatedGoals[goalIndex].objectives]To fully enforce the invariant that
|
||
| ) { | ||
| ;(updatedGoals[goalIndex].objectives[objIndex])[ | ||
| field as keyof EditableObjective | ||
| ] = value as never | ||
| const updatedObjectives = [...updatedGoals[goalIndex].objectives] | ||
| updatedObjectives[objIndex] = { | ||
| ...updatedObjectives[objIndex], | ||
| [field]: value, | ||
| } | ||
| updatedGoals[goalIndex] = { | ||
| ...updatedGoals[goalIndex], | ||
| objectives: updatedObjectives, | ||
| } | ||
| setEditingPlanData((prev: FormUpdatePlanData | null) => | ||
| prev ? { ...prev, goals: updatedGoals } : null, | ||
| ) | ||
| } | ||
| } else { | ||
| const updatedNewGoals = JSON.parse( | ||
| JSON.stringify(newPlanData.goals), | ||
| ) as ClientSideNewGoal[] | ||
| const updatedNewGoals = [...newPlanData.goals] | ||
| if ( | ||
| updatedNewGoals[goalIndex] && | ||
| updatedNewGoals[goalIndex].objectives && | ||
| updatedNewGoals[goalIndex].objectives[objIndex] | ||
| ) { | ||
| ;( | ||
| updatedNewGoals[goalIndex].objectives[ | ||
| objIndex | ||
| ] | ||
| )[field] = value as never | ||
| const updatedObjectives = [...updatedNewGoals[goalIndex].objectives] | ||
| updatedObjectives[objIndex] = { | ||
| ...updatedObjectives[objIndex], | ||
| [field]: value, | ||
| } | ||
| updatedNewGoals[goalIndex] = { | ||
| ...updatedNewGoals[goalIndex], | ||
| objectives: updatedObjectives, | ||
| } | ||
| setNewPlanData((prev: FormNewPlanData) => ({ | ||
| ...prev, | ||
| goals: updatedNewGoals, | ||
|
|
@@ -343,21 +355,27 @@ const TreatmentPlanManager: FC = () => { | |
| isEdit = false, | ||
| ) => { | ||
| if (isEdit && editingPlanData) { | ||
| const updatedGoals = JSON.parse( | ||
| JSON.stringify(editingPlanData.goals || []), | ||
| ) as EditableGoal[] | ||
| const updatedGoals = [...(editingPlanData.goals || [])] | ||
| if (updatedGoals[goalIndex] && updatedGoals[goalIndex].objectives) { | ||
| updatedGoals[goalIndex].objectives.splice(objIndex, 1) | ||
| const updatedObjectives = [...updatedGoals[goalIndex].objectives] | ||
| updatedObjectives.splice(objIndex, 1) | ||
| updatedGoals[goalIndex] = { | ||
| ...updatedGoals[goalIndex], | ||
| objectives: updatedObjectives, | ||
| } | ||
| setEditingPlanData((prev: FormUpdatePlanData | null) => | ||
| prev ? { ...prev, goals: updatedGoals } : null, | ||
| ) | ||
| } | ||
| } else { | ||
| const updatedNewGoals = JSON.parse( | ||
| JSON.stringify(newPlanData.goals), | ||
| ) as ClientSideNewGoal[] | ||
| const updatedNewGoals = [...newPlanData.goals] | ||
| if (updatedNewGoals[goalIndex] && updatedNewGoals[goalIndex].objectives) { | ||
| updatedNewGoals[goalIndex].objectives.splice(objIndex, 1) | ||
| const updatedObjectives = [...updatedNewGoals[goalIndex].objectives] | ||
| updatedObjectives.splice(objIndex, 1) | ||
| updatedNewGoals[goalIndex] = { | ||
| ...updatedNewGoals[goalIndex], | ||
| objectives: updatedObjectives, | ||
| } | ||
| setNewPlanData((prev: FormNewPlanData) => ({ | ||
| ...prev, | ||
| goals: updatedNewGoals, | ||
|
|
@@ -409,7 +427,10 @@ const TreatmentPlanManager: FC = () => { | |
| } | ||
| await fetchPlans() | ||
| setIsCreateModalOpen(false) | ||
| setNewPlanData(JSON.parse(JSON.stringify(initialNewPlanData) as unknown)) | ||
| setNewPlanData({ | ||
| ...initialNewPlanData, | ||
| goals: [...initialNewPlanData.goals], | ||
| } as FormNewPlanData) | ||
| toast.success('Treatment plan created successfully!') | ||
| } catch (err: unknown) { | ||
| const errorMessage = | ||
|
|
@@ -501,18 +522,21 @@ const TreatmentPlanManager: FC = () => { | |
| ? new Date(plan.startDate).toISOString().split('T')[0] | ||
| : '', | ||
| goals: plan.goals | ||
| ? JSON.parse( | ||
| JSON.stringify( | ||
| plan.goals.map((g) => ({ ...g, objectives: g.objectives || [] })), | ||
| ), | ||
| ) | ||
| : [], // Deep copy goals, ensure objectives is array | ||
| ? plan.goals.map((g) => ({ | ||
| ...g, | ||
| objectives: g.objectives ? [...g.objectives] : [], | ||
|
Comment on lines
524
to
+527
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. question (bug_risk): This used to deep-clone each goal and objective; now only goals and their |
||
| })) | ||
| : [], // Shallow copy goals, ensure objectives is array | ||
| } as FormUpdatePlanData) // Cast to ensure type compatibility | ||
| setIsEditModalOpen(true) | ||
| } | ||
|
|
||
| const openCreateModal = () => { | ||
| setNewPlanData(JSON.parse(JSON.stringify(initialNewPlanData) as unknown)) // Reset with deep copy | ||
| // β‘ Bolt: Replace JSON.parse/stringify with shallow copy for performance | ||
| setNewPlanData({ | ||
| ...initialNewPlanData, | ||
| goals: [...initialNewPlanData.goals], | ||
| } as FormNewPlanData) // Reset with shallow copy | ||
| setIsCreateModalOpen(true) | ||
| } | ||
|
|
||
|
|
||
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.
initialNewPlanDatais instantiated once at module load time (it containsstartDate: new Date()...). Resetting form state by spreadinginitialNewPlanDatameans the βdefaultβstartDatecan become stale (e.g., leaving the tab open overnight, then opening the create modal).Also, the repeated
as FormNewPlanDataassertions make it easier to accidentally drift from the real shape without TypeScript helping you.Suggestion
Introduce a factory function and reuse it for initialization + resets so
startDateis always fresh and you can avoidas:Then replace the reset call sites with
setNewPlanData(makeInitialNewPlanData()).Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.