fix: establish a clean tsconfig.json
#625
Conversation
|
✅ No documentation updates required. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR enables noUncheckedIndexedAccess in tsconfig.json to improve type safety across the codebase, requiring explicit null checks for array and object access.
- Added
@ts-expect-errorcomments inPeriodPicker.tsxfor date string splitting, consider using a more type-safe date parsing approach - Changed module resolution to
bundlerand module topreservein tsconfig.json, which needs careful testing for build compatibility - Marked
isolatedModulesandverbatimModuleSyntaxas "resolve ASAP" but left disabled, should be addressed soon - Added
ReadonlyArrayWithAtLeastOnetype inReports.tsxto ensure at least one report type is available - Improved color mapping safety in
DetailedTable.tsxandProfitAndLossSummariesMiniChart.tsxwith nullish coalescing
31 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile
| if (rowState.splits[rowNumber]) { | ||
| rowState.splits[rowNumber].amount = newAmount | ||
| rowState.splits[rowNumber].inputValue = newDisplaying | ||
| } | ||
| if (rowState.splits[0]) { | ||
| rowState.splits[0].amount = remaining | ||
| rowState.splits[0].inputValue = formatMoney(remaining) | ||
| } |
There was a problem hiding this comment.
logic: State mutations should be avoided. Consider creating a new state object instead of directly modifying rowState.splits
| if (rowState.splits[rowNumber]) { | |
| rowState.splits[rowNumber].amount = newAmount | |
| rowState.splits[rowNumber].inputValue = newDisplaying | |
| } | |
| if (rowState.splits[0]) { | |
| rowState.splits[0].amount = remaining | |
| rowState.splits[0].inputValue = formatMoney(remaining) | |
| } | |
| const newSplits = [...rowState.splits] | |
| if (newSplits[rowNumber]) { | |
| newSplits[rowNumber] = { | |
| ...newSplits[rowNumber], | |
| amount: newAmount, | |
| inputValue: newDisplaying | |
| } | |
| } | |
| if (newSplits[0]) { | |
| newSplits[0] = { | |
| ...newSplits[0], | |
| amount: remaining, | |
| inputValue: formatMoney(remaining) | |
| } | |
| } |
| accountId: data[i].accountId, | ||
| accountId: data?.[i]?.accountId ?? '', | ||
| })) | ||
| return onSuccess?.(resultsWithIds) |
There was a problem hiding this comment.
logic: onSuccess callback is called with optional chaining but its type doesn't indicate it's optional
| return onSuccess?.(resultsWithIds) | |
| return onSuccess(resultsWithIds) |
| // @ts-expect-error Guaranteed to produce `YYYY-MM-DD` | ||
| start_date: startDate.toISOString().split('T')[0], | ||
|
|
||
| // @ts-expect-error Guaranteed to produce `YYYY-MM-DD` | ||
| end_date: endDate.toISOString().split('T')[0], |
There was a problem hiding this comment.
style: Using @ts-expect-error to bypass type checking is not ideal. Consider using a more type-safe approach like format(startDate, 'yyyy-MM-dd') from date-fns library which already exists in the project dependencies.
| > | ||
| {chartData.map((entry, index) => { | ||
| let fill: string | undefined = typeColorMapping[index].color | ||
| let fill: string | undefined = typeColorMapping[index]?.color |
There was a problem hiding this comment.
style: Consider adding a default color value if typeColorMapping[index] is undefined to maintain consistent visualization
| background: typeColorMapping[idx] ? typeColorMapping[idx].color : '#e6e6e6', | ||
| opacity: typeColorMapping[idx] ? typeColorMapping[idx].opacity : 1, |
There was a problem hiding this comment.
logic: The fallback color '#e6e6e6' should use DEFAULT_COLOR_MAPPING.color for consistency
| background: typeColorMapping[idx] ? typeColorMapping[idx].color : '#e6e6e6', | |
| opacity: typeColorMapping[idx] ? typeColorMapping[idx].opacity : 1, | |
| background: typeColorMapping[idx] ? typeColorMapping[idx].color : DEFAULT_COLOR_MAPPING.color, | |
| opacity: typeColorMapping[idx] ? typeColorMapping[idx].opacity : DEFAULT_COLOR_MAPPING.opacity, |
| return { | ||
| r: values[0], | ||
| g: values[1], | ||
| b: values[2], | ||
| r: values[0] ?? 0, | ||
| g: values[1] ?? 0, | ||
| b: values[2] ?? 0, | ||
| } |
There was a problem hiding this comment.
logic: Using nullish coalescing with 0 as fallback could mask invalid hex values. Consider throwing an error if values[0-2] are undefined since this would indicate an invalid hex color.
| } | ||
|
|
||
| const DEFAULT_COLOR_MAPPING = { | ||
| color: '#EEEEF0', |
There was a problem hiding this comment.
NIT: I think this one should work too:
color: 'var(--color-base-200)This commit is not intended to be final - there are many errors to be cleaned-up
imkesin
force-pushed
the
imkesin/clean-tsconfig
branch
from
bcec4a1 to
5a4f693
Compare
Description
I've wanted to improve this for a long time. One of the main motivations is to turn on
noUncheckedIndexedAccess(and this is responsible for almost all of the changes).