Skip to content

Commit 2537f67

Browse files
eran132claude
andcommitted
fix: resolve all 125 SonarCloud issues
Comprehensive code quality cleanup based on SonarCloud static analysis: - Refactored async constructor to static factory (useVehicleLocations) - Fixed JSX value leaks with proper boolean checks (BusToolTip, MapContent) - Fixed dead code from always-truthy template literal (WorstLinesChart) - Replaced deprecated APIs (Ant Design Option, MUI PaperProps) - Added Readonly<> to 18 component prop types - Replaced window with globalThis (6 files) - Used modern JS APIs (replaceAll, Number.parseInt, Number.isNaN, .at(), .some(), .toSorted(), codePointAt) - Fixed accessibility issues (aria-labels, button vs anchor, keyboard) - Removed commented-out code, redundant fragments, unused exports - Wrapped Context provider values in useMemo to prevent re-renders - Extracted nested components, ternaries, and template literals - Fixed array index keys with data-based keys Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 363519b commit 2537f67

33 files changed

Lines changed: 173 additions & 174 deletions

src/api/siriService.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ export async function getSiriStopHitTimesAsync(
5050
const locationsByRideId = locations.reduce(
5151
(acc, location) => {
5252
if (location.siriRideId) {
53-
;(acc[location.siriRideId.toString()] ||= []).push({
53+
const key = location.siriRideId.toString()
54+
acc[key] ??= []
55+
acc[key].push({
5456
...location,
5557
longitude: location.lon || 0,
5658
latitude: location.lat || 0,
@@ -67,6 +69,10 @@ export async function getSiriStopHitTimesAsync(
6769
const diffFromTargetStart = (location: EnrichedLocation): number =>
6870
Math.abs(timestamp.diff(dayjs(location.recordedAtTime), 'second'))
6971

70-
const closestInTimeHits = stopHits.sort((a, b) => diffFromTargetStart(a) - diffFromTargetStart(b))
71-
return closestInTimeHits.sort((a, b) => a.recordedAtTime!.getTime() - b.recordedAtTime!.getTime())
72+
const closestInTimeHits = stopHits.toSorted(
73+
(a, b) => diffFromTargetStart(a) - diffFromTargetStart(b),
74+
)
75+
return closestInTimeHits.toSorted(
76+
(a, b) => a.recordedAtTime!.getTime() - b.recordedAtTime!.getTime(),
77+
)
7278
}

src/hooks/useSingleLineData.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ export const useSingleLineData = (
4646
useEffect(() => {
4747
if (!operatorId || !lineNumber) {
4848
setRoutes(undefined)
49-
setRouteKey(undefined)
50-
setStartTime(undefined)
49+
setRouteKey()
50+
setStartTime()
5151
setError(undefined)
5252
setSearch((prev) => ({
5353
...prev,
@@ -71,7 +71,7 @@ export const useSingleLineData = (
7171
if (err?.cause?.name !== 'AbortError') {
7272
setRoutes(undefined)
7373
setSearch((prev) => ({ ...prev, routes: undefined }))
74-
setRouteKey(undefined)
74+
setRouteKey()
7575
setError(err instanceof Error ? err.message : 'Failed to fetch routes')
7676
}
7777
})

src/hooks/useVehicleLocations.ts

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,18 @@ const loadedLocations = new Map<
4141
* it also caches the data, so if the same interval is requested again, it will not load it again.
4242
*/
4343
class LocationObservable {
44-
constructor(query: VehicleLocationQuery) {
45-
this.#loadData(query)
44+
static create(query: VehicleLocationQuery) {
45+
const instance = new LocationObservable()
46+
void instance.#loadData(query)
47+
return instance
4648
}
4749

4850
data: SiriVehicleLocationWithRelatedPydanticModel[] = []
4951
loading = true
5052

5153
async #loadData(querys: VehicleLocationQuery) {
5254
let offset = 0
53-
for (let i = 1; this.loading; i++) {
55+
while (this.loading) {
5456
const data = await fetchWithQueue(querys, offset)
5557
if (!data || data.length === 0) {
5658
this.loading = false
@@ -139,7 +141,10 @@ function getLocations(
139141
) {
140142
const key = `${formatTime(from)}-${formatTime(to)}-${operatorRef}-${lineRef}-${vehicleRef}`
141143
if (!loadedLocations.has(key)) {
142-
loadedLocations.set(key, new LocationObservable({ from, to, lineRef, vehicleRef, operatorRef }))
144+
loadedLocations.set(
145+
key,
146+
LocationObservable.create({ from, to, lineRef, vehicleRef, operatorRef }),
147+
)
143148
}
144149
const observable = loadedLocations.get(key)!
145150
return observable.observe(onUpdate)
@@ -171,6 +176,24 @@ export default function useVehicleLocations({
171176
}) {
172177
const [locations, setLocations] = useState<SiriVehicleLocationWithRelatedPydanticModel[]>([])
173178
const [isLoading, setIsLoading] = useState<boolean[]>([])
179+
180+
const handleFinished = (i: number) => {
181+
setIsLoading((prev) => {
182+
const newIsLoading = [...prev]
183+
newIsLoading[i] = false
184+
return newIsLoading
185+
})
186+
}
187+
188+
const handleData = (data: SiriVehicleLocationWithRelatedPydanticModel[]) => {
189+
setLocations((prev) =>
190+
uniqBy<SiriVehicleLocationWithRelatedPydanticModel>(
191+
[...prev, ...data].sort((a, b) => (a.id || 0) - (b.id || 0)),
192+
(loc) => loc.id,
193+
),
194+
)
195+
}
196+
174197
useEffect(() => {
175198
if (pause) return
176199
const range = split ? getMinutesInRange(from, to, split) : [{ from, to }]
@@ -184,18 +207,9 @@ export default function useVehicleLocations({
184207
operatorRef,
185208
onUpdate: (data) => {
186209
if ('finished' in data) {
187-
setIsLoading((prev) => {
188-
const newIsLoading = [...prev]
189-
newIsLoading[i] = false
190-
return newIsLoading
191-
})
210+
handleFinished(i)
192211
} else {
193-
setLocations((prev) =>
194-
uniqBy<SiriVehicleLocationWithRelatedPydanticModel>(
195-
[...prev, ...data].sort((a, b) => (a.id || 0) - (b.id || 0)),
196-
(loc) => loc.id,
197-
),
198-
)
212+
handleData(data)
199213
}
200214
},
201215
}),
@@ -208,8 +222,6 @@ export default function useVehicleLocations({
208222
}, [from, to, lineRef, vehicleRef, split])
209223
return {
210224
locations,
211-
isLoading: isLoading.some((loading) => loading),
225+
isLoading: isLoading.some(Boolean),
212226
}
213227
}
214-
215-
export {}

src/pages/DataResearch/DataResearch.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ const StackedResearchChart = ({
183183
}
184184

185185
return {
186-
data: result.sort((a, b) => a.ts - b.ts),
186+
data: result.toSorted((a, b) => a.ts - b.ts),
187187
operators: Array.from(operatorSet),
188188
}
189189
}, [filteredGraphData, field])

src/pages/about/index.tsx

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ const Privacy = () => {
7575
<Widget title={t('privacy')}>
7676
<p>
7777
<Trans i18nKey="aboutPage.privacyText">
78-
<a href={googlAnalyticsUrl}></a>
79-
<a href={googleAnaliticsPrivacyUrl}></a>
78+
<a href={googlAnalyticsUrl} aria-label="Google Analytics"></a>
79+
<a href={googleAnaliticsPrivacyUrl} aria-label="Google Analytics Privacy"></a>
8080
</Trans>
8181
</p>
8282
</Widget>
@@ -93,8 +93,8 @@ const License = () => {
9393
<Trans
9494
i18nKey="aboutPage.licenseInfo.text"
9595
values={{ licenseName: t('aboutPage.licenseInfo.licenseName') }}>
96-
<a href={licenseLink}></a>
97-
<a href={licenseOrgLink}></a>
96+
<a href={licenseLink} aria-label="Creative Commons License"></a>
97+
<a href={licenseOrgLink} aria-label="Creative Commons"></a>
9898
</Trans>
9999
</p>
100100
</Widget>
@@ -186,24 +186,25 @@ const Contributors = () => {
186186
{t('aboutPage.contributorsText')}
187187
<br />
188188
<Trans i18nKey="aboutPage.contributorsReadMore">
189-
<a href="https://github.com/hasadna/open-bus-map-search/blob/main/CONTRIBUTING.md"></a>
189+
<a
190+
href="https://github.com/hasadna/open-bus-map-search/blob/main/CONTRIBUTING.md"
191+
aria-label="Contributing guide"></a>
190192
</Trans>
191193
</p>
192194
<ol className="contributions">
193195
{isLoading && <p>Loading...</p>}
194196
{isError && <p>Error...</p>}
195-
{contributors &&
196-
contributors.map((author) => (
197-
<li key={author.id}>
198-
<a href={author.html_url}>
199-
<h2>{author.login}</h2>
200-
<img src={author.avatar_url} alt={author.login} />
201-
<p>
202-
{author.contributions} {t('aboutPage.contributions')}
203-
</p>
204-
</a>
205-
</li>
206-
))}
197+
{contributors?.map((author) => (
198+
<li key={author.id}>
199+
<a href={author.html_url}>
200+
<h2>{author.login}</h2>
201+
<img src={author.avatar_url} alt={author.login} />
202+
<p>
203+
{author.contributions} {t('aboutPage.contributions')}
204+
</p>
205+
</a>
206+
</li>
207+
))}
207208
</ol>
208209
</Widget>
209210
)
@@ -263,7 +264,7 @@ function useContributions() {
263264
.filter((a) => a.type === 'User')
264265
// sort by contributions
265266
.sort((a: Author, b: Author) => b.contributions - a.contributions)
266-
.reduce(combineAuthor, [] as Author[])
267+
.reduce((authors, author) => combineAuthor(authors, author), [] as Author[])
267268
return { contributors, isLoading, isError }
268269
} catch (error) {
269270
console.error(error)
@@ -274,10 +275,10 @@ function useContributions() {
274275
// sum contributions of the same user
275276
function combineAuthor(authors: Author[], author: Author) {
276277
const sameUser = authors.find((a) => a.login === author.login)
277-
if (!sameUser) {
278-
authors.push(author)
279-
} else {
278+
if (sameUser) {
280279
sameUser.contributions += author.contributions
280+
} else {
281+
authors.push(author)
281282
}
282283
return authors
283284
}

src/pages/bugReport/BugReportForm.tsx

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,12 @@ import './BugReportForm.scss'
1414
const BugReportForm = () => {
1515
const { t, i18n } = useTranslation()
1616
const [form] = Form.useForm<CreateIssuePostRequest>()
17-
// const [fileList, setFileList] = useState<UploadFile[]>([])
18-
1917
const mutation = useMutation({
2018
mutationFn: (values: CreateIssuePostRequest) =>
2119
ISSUES_API.issuesCreatePost({ createIssuePostRequest: values }),
2220
onSuccess: (response) => {
2321
if (response.data?.state === 'open') {
2422
form.resetFields()
25-
// setFileList([])
2623
}
2724
},
2825
onError: (error) => {
@@ -35,10 +32,6 @@ const BugReportForm = () => {
3532
mutation.mutate(values)
3633
}
3734

38-
// const onFileChange = (info: UploadChangeParam) => {
39-
// setFileList(info.fileList)
40-
// }
41-
4235
const options = useMemo(() => {
4336
return [
4437
{ value: 'always', label: t('bug_frequency.always') },
@@ -82,15 +75,16 @@ const BugReportForm = () => {
8275
onFinish={(values) => {
8376
onFinish(values)
8477
}}
85-
// onFinishFailed={onFinishFailed}
8678
labelCol={{ span: 6 }}
8779
wrapperCol={{ span: 18 }}>
8880
<Form.Item label={t('bug_type')} name="type" rules={[{ required: true }]}>
89-
<Select>
90-
<Select.Option value="bug">{t('bug_type_bug')}</Select.Option>
91-
<Select.Option value="feature">{t('bug_type_feature')}</Select.Option>
92-
<Select.Option value="other">{t('bug_type_other')}</Select.Option>
93-
</Select>
81+
<Select
82+
options={[
83+
{ value: 'bug', label: t('bug_type_bug') },
84+
{ value: 'feature', label: t('bug_type_feature') },
85+
{ value: 'other', label: t('bug_type_other') },
86+
]}
87+
/>
9488
</Form.Item>
9589

9690
<Form.Item
@@ -146,13 +140,7 @@ const BugReportForm = () => {
146140
label={t('bug_reproducibility')}
147141
name="reproducibility"
148142
rules={[{ required: true, min: 1, max: 100 }]}>
149-
<Select>
150-
{options.map((option) => (
151-
<Select.Option key={option.value} value={option.value}>
152-
{option.label}
153-
</Select.Option>
154-
))}
155-
</Select>
143+
<Select options={options} />
156144
</Form.Item>
157145

158146
<EasterEgg code="debug" autohide={false} onShow={() => form.setFieldValue('debug', true)}>
@@ -161,20 +149,6 @@ const BugReportForm = () => {
161149
</Form.Item>
162150
</EasterEgg>
163151

164-
{/* <Form.Item label={t('bug_attachments')} name="attachments">
165-
<Upload
166-
multiple
167-
maxCount={10}
168-
beforeUpload={() => false}
169-
listType="picture"
170-
fileList={fileList}
171-
onChange={onFileChange}>
172-
<Button icon={<FileUploadOutlined fontSize="small" />}>
173-
{t('bug_attachments_upload_button')}
174-
</Button>
175-
</Upload>
176-
</Form.Item> */}
177-
178152
<Form.Item>
179153
<Button type="primary" htmlType="submit" loading={mutation.isPending} dir={i18n.dir()}>
180154
{t('bug_submit')}

src/pages/components/YoutubeModal.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// import InfoOutlinedIcon from '@mui/icons-material/InfoOutlined'; //TODO:
21
import { InfoCircleOutlined } from '@ant-design/icons'
32
import { Typography } from '@mui/material'
43
import { Modal } from 'antd'
@@ -36,7 +35,7 @@ const InfoYoutubeModal = ({ videoUrl, label, title }: InfoYoutubeModalProps) =>
3635
{title}
3736
</Typography>
3837
<div className="modal-iframe-container">
39-
<iframe allowFullScreen src={videoUrl} />
38+
<iframe allowFullScreen src={videoUrl} title={title} />
4039
</div>
4140
</Modal>
4241
</>

src/pages/components/map-related/MapContent.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export function MapContent({ positions, plannedRouteStops, showNavigationButtons
9696
}}
9797
position={pos.loc}
9898
icon={icon}
99-
key={i}>
99+
key={pos.point?.id ?? `pos-${i}`}>
100100
<Popup minWidth={300} maxWidth={700}>
101101
<BusToolTip position={pos} icon={busIconPath(pos.operator!)}>
102102
{showNavigationButtons && (
@@ -112,7 +112,7 @@ export function MapContent({ positions, plannedRouteStops, showNavigationButtons
112112
)
113113
})}
114114

115-
{plannedRouteStops?.length && (
115+
{!!plannedRouteStops?.length && (
116116
<Polyline
117117
pathOptions={{ color: plannedRouteLineColor }}
118118
positions={plannedRouteStops.map((stop) => [
@@ -121,14 +121,14 @@ export function MapContent({ positions, plannedRouteStops, showNavigationButtons
121121
])}
122122
/>
123123
)}
124-
{plannedRouteStops?.length &&
124+
{!!plannedRouteStops?.length &&
125125
plannedRouteStops.map((stop) => {
126126
const { latitude, longitude } = stop.location
127127
return (
128128
<Marker key={stop.key} position={[latitude, longitude]} icon={plannedRouteStopMarker} />
129129
)
130130
})}
131-
{positions.length && (
131+
{positions.length > 0 && (
132132
<Polyline
133133
pathOptions={{ color: actualRouteLineColor }}
134134
positions={positions.map((position) => position.loc)}

src/pages/components/map-related/MapLayers/BusToolTip.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export function BusToolTip({ position, icon, children }: BusToolTipProps) {
171171
data={position}
172172
name={t('line')}
173173
/>
174-
{route?.id && (
174+
{route?.id != null && (
175175
<CustomTreeView<GtfsRoutePydanticModel>
176176
id={route?.id.toString()}
177177
data={route}

0 commit comments

Comments
 (0)