fix: enable no-floating-promises lint rule and fix all violations#1505
fix: enable no-floating-promises lint rule and fix all violations#1505eran132 wants to merge 1 commit intohasadna:mainfrom
Conversation
Enabled the @typescript-eslint/no-floating-promises rule (was disabled) and fixed all 16 violations across 13 files. Most were fire-and-forget calls (i18n.changeLanguage, navigate, prefetch) marked with `void`. One test assertion was missing `await`. Closes hasadna#1446 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Enables the @typescript-eslint/no-floating-promises rule and updates call sites across the app/tests to explicitly await promises or mark intentional fire-and-forget promises with void.
Changes:
- Turned on
@typescript-eslint/no-floating-promisesas an error in ESLint. - Added
awaitin a Playwright assertion to properly handle the returned promise. - Marked multiple async calls as intentionally un-awaited using
void(navigation, i18n changes, data fetches).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
eslint.config.js |
Enables @typescript-eslint/no-floating-promises as an error. |
tests/bugReport.spec.ts |
Awaits a Playwright expect(...) assertion promise. |
src/pages/operator/OperatorRoutes.tsx |
Marks navigation call as intentionally un-awaited. |
src/pages/lineProfile/LineProfile.tsx |
Marks async route fetch + navigation promises as intentionally un-awaited. |
src/pages/gapsPatterns/useGapsList.ts |
Marks effect’s async fetch as intentionally un-awaited. |
src/pages/gapsPatterns/GapsPatternsPage.tsx |
Marks effect’s async loader as intentionally un-awaited. |
src/pages/components/map-related/MapLayers/ComplaintModal.tsx |
Marks data fetch as intentionally un-awaited. |
src/pages/components/OperatorSelector.tsx |
Marks operators fetch as intentionally un-awaited. |
src/locale/allTranslations.ts |
Marks i18n initialization promise as intentionally un-awaited. |
src/layout/header/HeaderLinks/HeaderLinks.tsx |
Marks navigation call as intentionally un-awaited. |
src/layout/ThemeContext.tsx |
Marks i18n language-change promises as intentionally un-awaited. |
src/hooks/useVehicleLocations.ts |
Marks constructor-started async load as intentionally un-awaited. |
src/hooks/useSingleLineData.ts |
Marks async effects as intentionally un-awaited. |
.storybook/preview.tsx |
Marks Storybook locale language-change as intentionally un-awaited. |
Comments suppressed due to low confidence (3)
src/pages/components/map-related/MapLayers/ComplaintModal.tsx:80
- The
getSiriRideWithRelated(...).then(...).finally(...)chain has no.catch(...). If the request rejects, this becomes an unhandled promise rejection; handle the error explicitly (and consider clearingsiriRide/ showing an error state) while still keepingsetIsLoading(false)in afinally.
useEffect(() => {
setIsLoading(true)
void getSiriRideWithRelated(
(position.point!.siriRouteId || 0).toString(),
(position.point!.siriRideVehicleRef || '').toString(),
(position.point!.siriRouteLineRef || 0).toString(),
)
.then((siriRideRes: SiriRideWithRelatedPydanticModel) => setSiriRide(siriRideRes))
.finally(() => setIsLoading(false))
}, [position])
src/locale/allTranslations.ts:24
i18n.init(...)returns a promise; marking it withvoidavoids the lint error but still leaves potential init failures as unhandled promise rejections at module load time. Please attach a.catch(...)(or otherwise handle/propagate init errors) so failures are surfaced deterministically.
void i18n.use(initReactI18next).init({
showSupportNotice: false,
resources: {
ar: { translation: translationsAR },
he: { translation: translationsHE },
en: { translation: translationsEN },
ru: { translation: translationsRU },
},
lng: savedLang, // Use saved language or default to Hebrew
fallbackLng: 'he',
})
src/layout/ThemeContext.tsx:66
i18n.changeLanguage(language)is intentionally ignored withvoid, but any rejection will currently be unhandled while the effect continues to update document metadata. Please attach a.catch(...)(and consider whether subsequent document updates should happen only after a successful language change).
useEffect(() => {
if (!language) return
void i18n.changeLanguage(language)
document.title = i18n.t('website_name')
document.documentElement.dir = i18n.dir()
document.documentElement.lang = language
dayjs.locale(language)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| useEffect(() => { | ||
| getOperators(filter).then(setOperators) | ||
| void getOperators(filter).then(setOperators) |
There was a problem hiding this comment.
getOperators(filter).then(setOperators) is being intentionally ignored with void, but rejections are still unhandled. If getAgencyList() fails this will surface as an unhandled promise rejection; please add explicit error handling (e.g., catch/log + optionally setOperators([]) or show an error state).
| void getOperators(filter).then(setOperators) | |
| void getOperators(filter) | |
| .then(setOperators) | |
| .catch((error) => { | |
| console.error('Failed to load operators', error) | |
| setOperators([]) | |
| }) |
| constructor(query: VehicleLocationQuery) { | ||
| this.#loadData(query) | ||
| void this.#loadData(query) | ||
| } |
There was a problem hiding this comment.
#loadData is kicked off as a fire-and-forget promise from the constructor. If fetchWithQueue throws after retries, the rejection is currently unhandled and loading/observer state won't be updated. Please catch errors from #loadData (e.g., set loading=false, notify observers, and/or log) to avoid unhandled promise rejections.
| (newLanguage: string) => { | ||
| setLanguage(newLanguage) | ||
| i18n.changeLanguage(newLanguage) | ||
| void i18n.changeLanguage(newLanguage) | ||
| }, |
There was a problem hiding this comment.
i18n.changeLanguage(...) returns a promise; using void satisfies the lint rule but any failure would become an unhandled promise rejection. Please add explicit error handling (e.g., .catch(...)) so language-switch failures don’t get swallowed.
|
|
||
| useEffect(() => { | ||
| i18n.changeLanguage(locale) | ||
| void i18n.changeLanguage(locale) |
There was a problem hiding this comment.
void i18n.changeLanguage(locale) intentionally ignores the returned promise, but failures will still be unhandled. Please attach a .catch(...) (and consider guarding locale if it can be undefined) to avoid unhandled promise rejections in Storybook.
| void i18n.changeLanguage(locale) | |
| if (!locale) { | |
| return | |
| } | |
| void i18n.changeLanguage(locale).catch((error) => { | |
| console.error('Failed to change Storybook language:', error) | |
| }) |
| } | ||
|
|
||
| fetchOptions() | ||
| void fetchOptions() |
There was a problem hiding this comment.
fetchOptions performs awaited API calls (e.g., getRoutesByLineRef) but has no try/catch. Since it’s invoked with void fetchOptions(), any rejection will be unhandled. Please add error handling inside fetchOptions (or attach a .catch) and ensure the UI state (options) is left in a consistent state on failure.
| void fetchOptions() | |
| void fetchOptions().catch((err) => { | |
| console.error(err) | |
| setOptions([]) | |
| }) |
| } | ||
| loadSearchData(signal) | ||
| void loadSearchData(signal) | ||
| return () => controller.abort() | ||
| }, [operatorId, lineNumber, endDate, startDate, setSearch]) |
There was a problem hiding this comment.
loadSearchData sets routesIsLoading to true, but if getRoutesAsync(...) rejects the function exits before calling setRoutesIsLoading(false), leaving the page stuck in a loading state and producing an unhandled promise rejection (since it’s called via void loadSearchData(...)). Please wrap the body in try/catch/finally to both handle errors and reliably reset routesIsLoading.
Summary
@typescript-eslint/no-floating-promisesESLint rule (was previously disabled)void(i18n.changeLanguage, navigate, prefetch calls)awaitCloses #1446
Test plan
🤖 Generated with Claude Code