Fix/ts expect errors and TODOs - #1668
Open
Arnei wants to merge 8 commits into
Open
Conversation
The current-view-first sort mutated link items with a fake tmpIndex property to work around missing typing, requiring two @ts-expect-error suppressions. Compute the sort key inline instead. This commit was largely AI generated
editableRef held whichever DOM/component instance was mounted for the current metadata field type (input, textarea, DatePicker, or react-select), so it was typed any with several eslint-disable-next-line comments to allow the unsafe member access. Since only one field variant ever mounts at a time, split it into four separately-typed refs and focus whichever one is set, removing the need for any unsafe casts. This commit was largely AI generated
The wrapper used React.ComponentProps<typeof FormikFastField>, which resolves to any since FormikFastField itself is declared as React.FC<any>. Type it via Formik's own FastFieldConfig, derived rather than hand-copied so it stays correct if Formik's config props change, while custom component/as props (which take app-specific extra props via sibling attributes) stay loosely typed like upstream Formik's own Field. This commit was largely AI generated
checkConflicts is an async thunk that resolves to
{ conflicts, hasSchedulingConflicts }, but submitForm checked
truthiness of the dispatched action object itself via .then(r => ...),
which is always truthy. This meant the form would proceed to save
scheduling changes even when a genuine conflict was detected. Use
.unwrap() to read the actual payload and branch on
hasSchedulingConflicts, and add a .catch() since .unwrap() turns a
rejected thunk into a real promise rejection that was previously
silently swallowed.
This commit was largely AI generated
Traced through git history: the "vastly different" data the TODO referred to was the old (commented-out, since removed) Angular reducer's empty-workflow object, which matched the full workflow- details shape. The current placeholder already matches our initial state exactly, and every consumer narrows on `"status" in workflow` before reading details-only fields, so the simpler fallback degrades safely. Replace the stale question with a note on why this is fine. This commit was largely AI generated
NewSourcePage.tsx already implements this: all three of its SchedulingTime usages branch on sourceMode === "SCHEDULE_MULTIPLE" inside the callbackHour/callbackMinute props they pass in, calling the *Multiple variants from dateUtils.ts. That's the extension point SchedulingTime exposes, so the shared component itself never needed to know about "Multiple" mode - the comment was just left behind. This commit was largely AI generated
The two @ts-expect-error TS(7006) lines were leftovers from an automated JS->TS migration tool, blanket-suppressing the entire assignment with no explanation. The actual issue is a real TypeScript limitation: it can't correlate a generic `keyof EditedEvents` key with its value's type across separate property accesses, even though `typeof value === "string"` has already verified it at runtime. Scope the escape hatch down to just the value being assigned via `as never`, with a comment explaining why, so it can't silently hide an unrelated future type error the way suppressing the whole line could. This commit was largely AI generated
seriesDetailsSlice.ts's fetchSeriesStatistics/fetchSeriesStatisticsValueUpdate use the exact same pattern as these two (createAppAsyncThunk wrapping the shared fetchStatistics/fetchStatisticsValueUpdate helpers from statisticsSlice.ts), just swapping "episode" for "series" - and carry no such TODO. The modernization already happened; these comments were just never cleaned up. This commit was largely AI generated
Contributor
|
This pull request has conflicts ☹ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses various smaller todos and typing issues. Best reviewed commit by commit.
How to test this
Check that things still work, particularly metadata and scheduling
AI Usage
Claude Sonnet 5 was used for this.