From 433ba20d41fdaffb51e5ec6239643ee99cd7c86b Mon Sep 17 00:00:00 2001 From: Howard Wilson Date: Tue, 20 Oct 2026 19:05:51 +0200 Subject: [PATCH 1/2] Type Spotify API responses and remove any Add hand-written types in src/spotify.ts for the responses the app reads, checked against the recorded fixtures through the typed test builders. Type the export config, the Liked Songs pseudo-playlist and the unloaded playlist placeholder, and re-enable @typescript-eslint/no-explicit-any. Use a btn-xs class and inline Font Awesome icon tuples instead of the @ts-expect-error workarounds. Blank the artist names of podcast episodes, which Spotify sends as null and the CSV wrote out as "null". Co-Authored-By: Claude Opus 5.5 --- eslint.config.js | 2 - src/App.tsx | 19 ++- src/auth.ts | 16 +- src/components/ConfigDropdown.tsx | 8 +- src/components/Paginator.tsx | 4 +- src/components/PlaylistExporter.test.ts | 11 +- src/components/PlaylistExporter.tsx | 32 ++-- src/components/PlaylistRow.tsx | 21 ++- src/components/PlaylistSearch.tsx | 2 +- src/components/PlaylistTable.test.tsx | 7 +- src/components/PlaylistTable.tsx | 17 +- src/components/PlaylistsExporter.tsx | 8 +- .../__snapshots__/PlaylistTable.test.tsx.snap | 6 +- src/components/data/PlaylistsData.test.ts | 2 +- src/components/data/PlaylistsData.ts | 55 ++++-- src/components/data/TracksAlbumData.ts | 21 +-- src/components/data/TracksArtistsData.ts | 26 +-- .../data/TracksAudioFeaturesData.ts | 13 +- src/components/data/TracksBaseData.ts | 29 +++- src/components/data/TracksData.test.ts | 21 ++- src/components/data/TracksData.ts | 4 +- src/helpers.test.ts | 20 ++- src/helpers.ts | 33 ++-- src/mocks/handlers.ts | 7 +- src/spotify.ts | 157 ++++++++++++++++++ src/test/bugsnag.ts | 4 +- src/test/fixtures/spotify/builders.test.ts | 7 +- src/test/fixtures/spotify/builders.ts | 56 ++++--- src/test/fixtures/spotify/scrub.ts | 10 +- 29 files changed, 435 insertions(+), 183 deletions(-) create mode 100644 src/spotify.ts diff --git a/eslint.config.js b/eslint.config.js index a7dfa551..07c5b006 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -14,8 +14,6 @@ export default tseslint.config( tseslint.configs.recommended, { rules: { - // Spotify responses are untyped for now: TASK-24.1 re-enables this - "@typescript-eslint/no-explicit-any": "off", "@typescript-eslint/no-unused-vars": ["error", { ignoreRestSiblings: true }], "@typescript-eslint/no-empty-object-type": ["error", { allowInterfaces: "with-single-extends" }], }, diff --git a/src/App.tsx b/src/App.tsx index 247973c3..0ed344d5 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -3,6 +3,7 @@ import "./icons" import React, { useEffect, useState } from 'react' import Bugsnag from "@bugsnag/js" +import { AxiosResponse, isAxiosError } from "axios" import { useTranslation, Translation } from "react-i18next" import { Alert } from "react-bootstrap" import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' @@ -10,7 +11,7 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' import Login from 'components/Login' import PlaylistTable from "components/PlaylistTable" import TopMenu from "components/TopMenu" -import { loadAccessToken, exchangeCodeForToken, MissingCodeVerifierError } from "auth" +import { loadAccessToken, exchangeCodeForToken, MissingCodeVerifierError, OAuthErrorResponse } from "auth" const baseUrl = import.meta.env.BASE_URL @@ -24,8 +25,12 @@ function loginErrorFrom(params: URLSearchParams): LoginError | null { return error === "access_denied" ? { kind: "cancelled" } : { kind: "failed", detail: `Spotify: ${error}` } } -function describeExchangeFailure(error: any): string { - const spotifyError = error.response?.data +function oauthResponse(error: unknown) { + return isAxiosError>(error) ? error.response : undefined +} + +function describeExchangeFailure(error: Error): string { + const spotifyError = oauthResponse(error)?.data if (!spotifyError?.error) { return error.message } return spotifyError.error_description @@ -34,7 +39,7 @@ function describeExchangeFailure(error: any): string { } // Sends Spotify's reason but never the request, whose body holds the code and verifier -function reportLoginFailure(error: Error, detail: string, response?: any) { +function reportLoginFailure(error: Error, detail: string, response?: AxiosResponse>) { // Google's Read Aloud fetcher replays callback URLs with empty storage, so its failures are noise if (navigator.userAgent.includes("Google-Read-Aloud")) { return } @@ -70,7 +75,7 @@ function App() { let view - const onSetSubtitle = (subtitle: any) => { + const onSetSubtitle = (subtitle: React.JSX.Element) => { setSubtitle(subtitle) } @@ -84,9 +89,9 @@ function App() { if (callbackError?.detail) { reportLoginFailure(new Error(`Spotify authorization error: ${params.get("error")}`), callbackError.detail) } else if (code && !loadAccessToken()) { - exchangeCodeForToken(code).then(setAccessToken, (error) => { + exchangeCodeForToken(code).then(setAccessToken, (error: Error) => { const detail = describeExchangeFailure(error) - reportLoginFailure(error, detail, error.response) + reportLoginFailure(error, detail, oauthResponse(error)) setLoginError({ kind: error instanceof MissingCodeVerifierError ? "wrong_browser" : "failed", detail }) }).finally(() => setExchangingCode(false)) } diff --git a/src/auth.ts b/src/auth.ts index c074e376..24b01465 100644 --- a/src/auth.ts +++ b/src/auth.ts @@ -3,7 +3,7 @@ // - https://developer.spotify.com/documentation/web-api/tutorials/code-pkce-flow import Bugsnag from "@bugsnag/js" -import axios from "axios" +import axios, { AxiosResponse } from "axios" import { redirectTo } from "./navigation" const SPOTIFY_CLIENT_ID = "9950ac751e34487dbbe027c4fd7f8e99" @@ -13,6 +13,7 @@ const SPOTIFY_SCOPES = "playlist-read-private playlist-read-collaborative user-l const EXPIRY_MARGIN = 60 * 1000 type TokenResponse = { access_token: string, expires_in: number, refresh_token?: string } +export type OAuthErrorResponse = { error: string, error_description?: string } // Access token management export function loadAccessToken(): string | null { @@ -55,7 +56,7 @@ async function requestRefresh(): Promise { if (!refreshToken) { throw new SessionExpiredError() } try { - const response = await axios.post( + const response = await axios.post( SPOTIFY_TOKEN_URL, new URLSearchParams({ grant_type: "refresh_token", @@ -66,18 +67,17 @@ async function requestRefresh(): Promise { ) saveTokens(response.data) return response.data.access_token - } catch (error: any) { + } catch (error) { // With no response it's the connection, which the request's own retries can ride out - if (!error.response) { throw error } + if (!axios.isAxiosError>(error) || !error.response) { throw error } - reportRefreshFailure(error) + reportRefreshFailure(error.response) throw new SessionExpiredError() } } // Sends Spotify's reason but never the request, whose body holds the refresh token -function reportRefreshFailure(error: any) { - const { status, data } = error.response +function reportRefreshFailure({ status, data }: AxiosResponse>) { const reason = data?.error_description ? `${data.error} (${data.error_description})` : data?.error ?? status Bugsnag.notify(new Error(`Token refresh failed: ${reason}`), (event) => { @@ -173,7 +173,7 @@ export async function exchangeCodeForToken(code: string): Promise { throw new MissingCodeVerifierError() } - const response = await axios.post( + const response = await axios.post( SPOTIFY_TOKEN_URL, new URLSearchParams({ client_id: clientId, diff --git a/src/components/ConfigDropdown.tsx b/src/components/ConfigDropdown.tsx index 98c824e2..5fe2612f 100644 --- a/src/components/ConfigDropdown.tsx +++ b/src/components/ConfigDropdown.tsx @@ -5,8 +5,14 @@ import { withTranslation, WithTranslation } from "react-i18next" import { Dropdown, Form } from "react-bootstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" +export interface ExportConfig { + includeArtistsData: boolean + includeAudioFeaturesData: boolean + includeAlbumData: boolean +} + interface ConfigDropdownProps extends WithTranslation { - onConfigChanged: (config: any) => void + onConfigChanged: (config: ExportConfig) => void } class ConfigDropdown extends React.Component { diff --git a/src/components/Paginator.tsx b/src/components/Paginator.tsx index 76dbcb69..8349920e 100644 --- a/src/components/Paginator.tsx +++ b/src/components/Paginator.tsx @@ -8,13 +8,13 @@ interface PaginatorProps { } class Paginator extends React.Component { - nextClick = (e: any) => { + nextClick = (e: React.MouseEvent) => { e.preventDefault() this.props.onPageChanged(this.props.currentPage + 1) } - prevClick = (e: any) => { + prevClick = (e: React.MouseEvent) => { e.preventDefault() this.props.onPageChanged(this.props.currentPage - 1) diff --git a/src/components/PlaylistExporter.test.ts b/src/components/PlaylistExporter.test.ts index 57e1d94a..7255a88a 100644 --- a/src/components/PlaylistExporter.test.ts +++ b/src/components/PlaylistExporter.test.ts @@ -4,6 +4,7 @@ import { pagedGet, spotifyGet } from "../mocks/handlers" import { csvHeaders, csvRecords } from "../test/csv" import { API, buildArtist, buildPlaylist, buildPlaylistItem, buildTrack, buildUser } from "../test/fixtures/spotify/builders" import PlaylistExporter from "./PlaylistExporter" +import { Track } from "../spotify" const baseHeaders = [ "Track URI", "Track Name", "Artist URI(s)", "Artist Name(s)", "Album URI", "Album Name", "Album Artist URI(s)", @@ -13,7 +14,9 @@ const baseHeaders = [ const playlist = buildPlaylist({ id: 'p1', name: 'Mixtape', tracks: { total: 1 } }) -const serveTracks = (...tracks: object[]) => +const noExtraData = { includeArtistsData: false, includeAudioFeaturesData: false, includeAlbumData: false } + +const serveTracks = (...tracks: Track[]) => server.use(pagedGet(`${API}/playlists/p1/tracks`, tracks.map(track => buildPlaylistItem({ track, added_by: buildUser({ id: 'dj' }), added_at: '2024-01-02T03:04:05Z' })))) beforeEach(() => { @@ -24,7 +27,7 @@ describe("PlaylistExporter", () => { test("puts base track columns before the item columns", async () => { serveTracks(buildTrack()) - const csv = await new PlaylistExporter(playlist, {}).csvData() + const csv = await new PlaylistExporter(playlist, noExtraData).csvData() expect(csvHeaders(csv)).toEqual(baseHeaders) }) @@ -51,7 +54,7 @@ describe("PlaylistExporter", () => { test("quotes every field so commas, quotes and newlines survive", async () => { serveTracks(buildTrack({ id: 't1', name: 'Say "Hi",\nthen leave', artists: [buildArtist({ name: 'A, B' })] })) - const csv = await new PlaylistExporter(playlist, {}).csvData() + const csv = await new PlaylistExporter(playlist, noExtraData).csvData() expect(csvRecords(csv)).toEqual([expect.objectContaining({ "Track URI": "spotify:track:t1", @@ -68,7 +71,7 @@ describe("PlaylistExporter", () => { ['tabs\tand "quotes"', 'tabs_and_quotes_'], ['Déjà Vu', 'déjà_vu'], ])("names the file for %j as %j", (name, fileName) => { - const exporter = new PlaylistExporter({ ...playlist, name }, {}) + const exporter = new PlaylistExporter({ ...playlist, name }, noExtraData) expect(exporter.fileName(false)).toBe(fileName) expect(exporter.fileName()).toBe(`${fileName}.csv`) diff --git a/src/components/PlaylistExporter.tsx b/src/components/PlaylistExporter.tsx index ddbcddb5..7b608e82 100644 --- a/src/components/PlaylistExporter.tsx +++ b/src/components/PlaylistExporter.tsx @@ -1,22 +1,24 @@ import { saveAs } from "file-saver" import i18n from "../i18n/config" -import TracksData from "components/data/TracksData" -import TracksBaseData from "components/data/TracksBaseData" +import TracksData, { CsvValue } from "components/data/TracksData" +import TracksBaseData, { TrackItem } from "components/data/TracksBaseData" import TracksArtistsData from "components/data/TracksArtistsData" import TracksAudioFeaturesData from "components/data/TracksAudioFeaturesData" import TracksAlbumData from "components/data/TracksAlbumData" +import { ExportablePlaylist } from "components/data/PlaylistsData" +import { ExportConfig } from "components/ConfigDropdown" class TracksCsvFile { - playlist: any - trackItems: any + playlist: ExportablePlaylist + trackItems: TrackItem[] columnNames: string[] - lineData: Map + lineData: Map lineTrackUris: string[] - lineTrackData: string[][] + lineTrackData: CsvValue[][] - constructor(playlist: any, trackItems: any) { + constructor(playlist: ExportablePlaylist, trackItems: TrackItem[]) { this.playlist = playlist this.trackItems = trackItems this.columnNames = [ @@ -25,8 +27,8 @@ class TracksCsvFile { ] this.lineData = new Map() - this.lineTrackUris = trackItems.map((i: any) => i.track.uri) - this.lineTrackData = trackItems.map((i: any) => [ + this.lineTrackUris = trackItems.map((i) => i.track.uri) + this.lineTrackData = trackItems.map((i) => [ i.added_by == null ? '' : i.added_by.uri, i.added_at ]) @@ -39,7 +41,7 @@ class TracksCsvFile { this.columnNames.push(...tracksData.dataLabels()) } - const data: Map = await tracksData.data() + const data = await tracksData.data() this.lineTrackUris.forEach((uri: string, index: number) => { if (data.has(uri)) { @@ -64,17 +66,17 @@ class TracksCsvFile { return csvContent } - sanitize(string: string): string { - return '"' + String(string).replace(/"/g, '""') + '"' + sanitize(value: CsvValue): string { + return '"' + String(value).replace(/"/g, '""') + '"' } } // Handles exporting a single playlist as a CSV file class PlaylistExporter { - playlist: any - config: any + playlist: ExportablePlaylist + config: ExportConfig - constructor(playlist: any, config: any) { + constructor(playlist: ExportablePlaylist, config: ExportConfig) { this.playlist = playlist this.config = config } diff --git a/src/components/PlaylistRow.tsx b/src/components/PlaylistRow.tsx index de9ce1f9..acad48a9 100644 --- a/src/components/PlaylistRow.tsx +++ b/src/components/PlaylistRow.tsx @@ -4,10 +4,12 @@ import { Button } from "react-bootstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" import PlaylistExporter from "./PlaylistExporter" +import { ExportablePlaylist, PlaylistPlaceholder } from "./data/PlaylistsData" +import { ExportConfig } from "./ConfigDropdown" interface PlaylistRowProps extends WithTranslation { - playlist: any, - config: any + playlist: ExportablePlaylist | PlaylistPlaceholder, + config: ExportConfig onApiError: (error: unknown) => void } @@ -16,12 +18,12 @@ class PlaylistRow extends React.Component { exporting: false } - exportPlaylist = () => { + exportPlaylist = (playlist: ExportablePlaylist) => { this.setState( { exporting: true }, () => { (new PlaylistExporter( - this.props.playlist, + playlist, this.props.config )).export().catch(this.props.onApiError).finally(() => { this.setState({ exporting: false }) @@ -30,7 +32,7 @@ class PlaylistRow extends React.Component { ) } - renderTickCross(condition: boolean) { + renderTickCross(condition?: boolean) { if (condition) { return } else { @@ -38,7 +40,7 @@ class PlaylistRow extends React.Component { } } - renderIcon(playlist: any) { + renderIcon(playlist: ExportablePlaylist | PlaylistPlaceholder) { if (playlist.id === 'liked') { return ; } else { @@ -48,7 +50,6 @@ class PlaylistRow extends React.Component { render() { const playlist = this.props.playlist - const icon = ['fas', (this.state.exporting ? 'sync' : 'download')] if (playlist.uri == null) return ( @@ -70,10 +71,8 @@ class PlaylistRow extends React.Component { {this.renderTickCross(playlist.public)} {this.renderTickCross(playlist.collaborative)} - {/* @ts-expect-error - custom xs size from index.scss */} - diff --git a/src/components/PlaylistSearch.tsx b/src/components/PlaylistSearch.tsx index 8ff621d6..4d035dca 100644 --- a/src/components/PlaylistSearch.tsx +++ b/src/components/PlaylistSearch.tsx @@ -7,7 +7,7 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" interface PlaylistSearchProps extends WithTranslation { onPlaylistSearch: (query: string) => void - onPlaylistSearchCancel: () => Promise + onPlaylistSearchCancel: () => Promise } class PlaylistSearch extends React.Component { diff --git a/src/components/PlaylistTable.test.tsx b/src/components/PlaylistTable.test.tsx index 96e02583..65e425af 100644 --- a/src/components/PlaylistTable.test.tsx +++ b/src/components/PlaylistTable.test.tsx @@ -7,6 +7,7 @@ import FileSaver from "file-saver" import JSZip from "jszip" import PlaylistTable from "./PlaylistTable" +import { ExportConfig } from "./ConfigDropdown" import "../icons" import { server } from "../mocks/server" @@ -112,8 +113,8 @@ const localTrack = (name: string, artist: string, durationMs: number) => ({ const requestedUrls = () => handlerCalled.mock.calls.map(([url]) => url) -const renderTable = (config = {}) => - render() +const renderTable = (config: Partial = {}) => + render() async function savedFile() { const saveAs = vi.mocked(FileSaver.saveAs) @@ -524,7 +525,7 @@ describe("when an export fails unexpectedly", () => { test("Export All stays busy until the zip is saved", async () => { let finishZip: (zip: Blob) => void = () => {} - generateAsync().mockReturnValue(new Promise(resolve => { finishZip = resolve }) as any) + generateAsync().mockReturnValue(new Promise(resolve => { finishZip = resolve })) renderTable() const button = await screen.findByRole('button', { name: /Export All/ }) diff --git a/src/components/PlaylistTable.tsx b/src/components/PlaylistTable.tsx index cb1913d8..a701fb2f 100644 --- a/src/components/PlaylistTable.tsx +++ b/src/components/PlaylistTable.tsx @@ -4,16 +4,17 @@ import { Alert, ProgressBar } from "react-bootstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" import Bugsnag from "@bugsnag/js" -import PlaylistsData from "./data/PlaylistsData" -import ConfigDropdown, { ConfigDropdownRef } from "./ConfigDropdown" +import PlaylistsData, { ExportablePlaylist, PlaylistPlaceholder } from "./data/PlaylistsData" +import ConfigDropdown, { ConfigDropdownRef, ExportConfig } from "./ConfigDropdown" import PlaylistSearch, { PlaylistSearchRef } from "./PlaylistSearch" import PlaylistRow from "./PlaylistRow" import Paginator from "./Paginator" import PlaylistsExporter from "./PlaylistsExporter" import { apiCall, apiCallErrorHandler, ApiError } from "helpers" +import { CurrentUser } from "spotify" interface PlaylistTableProps extends WithTranslation { - config?: any, + config?: ExportConfig, onSetSubtitle: (subtitile: React.JSX.Element) => void } @@ -29,7 +30,7 @@ class PlaylistTable extends React.Component { initialized: false, apiError: null as ApiError | null, searchQuery: "", - playlists: [], + playlists: [] as (ExportablePlaylist | PlaylistPlaceholder)[], playlistCount: 0, likedSongs: { limit: 0, @@ -162,7 +163,7 @@ class PlaylistTable extends React.Component { }) } - handleConfigChanged = (config: any) => { + handleConfigChanged = (config: ExportConfig) => { Bugsnag.leaveBreadcrumb(`Config updated to ${JSON.stringify(config)}`) this.setState({ config: config }) @@ -181,10 +182,10 @@ class PlaylistTable extends React.Component { async componentDidMount() { try { - const user = await apiCall("https://api.spotify.com/v1/me") + const user = await apiCall("https://api.spotify.com/v1/me") .then(response => response.data) - Bugsnag.setUser(user.id, user.uri, user.display_name) + Bugsnag.setUser(user.id, user.uri, user.display_name ?? undefined) this.userId = user.id this.playlistsData = new PlaylistsData( @@ -255,7 +256,7 @@ class PlaylistTable extends React.Component { - {this.state.playlists.map((playlist: any, i) => { + {this.state.playlists.map((playlist, i) => { return void onPlaylistsExportDone: () => void onApiError: (error: unknown) => void @@ -23,7 +24,7 @@ class PlaylistsExporter extends React.Component { exporting: false } - async export(playlistsData: PlaylistsData, searchQuery: string, config: any) { + async export(playlistsData: PlaylistsData, searchQuery: string, config: ExportConfig) { const playlistFileNames = new Set() const playlistCsvExports = new Array() @@ -77,8 +78,7 @@ class PlaylistsExporter extends React.Component { render() { const text = this.props.searchQuery === "" ? this.props.i18n.t("export_all") : this.props.i18n.t("export_search_results") - // @ts-expect-error - custom xs size from index.scss - return } diff --git a/src/components/__snapshots__/PlaylistTable.test.tsx.snap b/src/components/__snapshots__/PlaylistTable.test.tsx.snap index 835b582c..04e43e09 100644 --- a/src/components/__snapshots__/PlaylistTable.test.tsx.snap +++ b/src/components/__snapshots__/PlaylistTable.test.tsx.snap @@ -142,7 +142,7 @@ exports[`playlist loading 1`] = ` class="export text-end" >