From 2dee12e9e0dd4188b7873f81fee3dd6813b1bdcd Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Wed, 9 Jul 2025 11:25:45 -0700 Subject: [PATCH] Avoid non-serializable object in Redux store --- .../components/PlaylistWindow/TrackCell.tsx | 2 +- packages/webamp/js/reducers/playlist.ts | 41 +++++++++---------- packages/webamp/js/selectors.ts | 11 ++++- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/packages/webamp/js/components/PlaylistWindow/TrackCell.tsx b/packages/webamp/js/components/PlaylistWindow/TrackCell.tsx index 3a357eb6..8468629e 100644 --- a/packages/webamp/js/components/PlaylistWindow/TrackCell.tsx +++ b/packages/webamp/js/components/PlaylistWindow/TrackCell.tsx @@ -19,7 +19,7 @@ interface Props { function TrackCell({ children, handleMoveClick, index, id }: Props) { const skinPlaylistStyle = useTypedSelector(Selectors.getSkinPlaylistStyle); - const selectedTrackIds = useTypedSelector(Selectors.getSelectedTrackIds); + const selectedTrackIds = useTypedSelector(Selectors.getSelectedTrackIdsSet); const currentTrackId = useTypedSelector(Selectors.getCurrentTrackId); const selected = selectedTrackIds.has(id); const current = currentTrackId === id; diff --git a/packages/webamp/js/reducers/playlist.ts b/packages/webamp/js/reducers/playlist.ts index 3d7cf337..3a90da9d 100644 --- a/packages/webamp/js/reducers/playlist.ts +++ b/packages/webamp/js/reducers/playlist.ts @@ -5,24 +5,16 @@ export interface PlaylistState { trackOrder: number[]; lastSelectedIndex: number | null; currentTrack: number | null; - selectedTracks: Set; + selectedTracks: number[]; } const defaultPlaylistState: PlaylistState = { trackOrder: [], currentTrack: null, lastSelectedIndex: null, - selectedTracks: new Set(), + selectedTracks: [], }; -function toggleSetMembership(set: Set, value: T): void { - if (set.has(value)) { - set.delete(value); - } else { - set.add(value); - } -} - const playlist = ( state: PlaylistState = defaultPlaylistState, action: Action @@ -31,13 +23,18 @@ const playlist = ( case "CLICKED_TRACK": return { ...state, - selectedTracks: new Set([state.trackOrder[(action as any).index]]), + selectedTracks: [state.trackOrder[(action as any).index]], lastSelectedIndex: (action as any).index, }; case "CTRL_CLICKED_TRACK": { const id = state.trackOrder[(action as any).index]; - const newSelectedTracks = new Set(state.selectedTracks); - toggleSetMembership(newSelectedTracks, id); + const index = state.selectedTracks.indexOf(id); + const newSelectedTracks = [...state.selectedTracks]; + if (index === -1) { + newSelectedTracks.push(id); + } else { + newSelectedTracks.splice(index, 1); + } return { ...state, selectedTracks: newSelectedTracks, @@ -54,7 +51,7 @@ const playlist = ( const clickedIndex = (action as any).index; const start = Math.min(clickedIndex, state.lastSelectedIndex); const end = Math.max(clickedIndex, state.lastSelectedIndex); - const selectedTracks = new Set(state.trackOrder.slice(start, end + 1)); + const selectedTracks = state.trackOrder.slice(start, end + 1); return { ...state, selectedTracks, @@ -62,18 +59,18 @@ const playlist = ( case "SELECT_ALL": return { ...state, - selectedTracks: new Set(state.trackOrder), + selectedTracks: [...state.trackOrder], }; case "SELECT_ZERO": return { ...state, - selectedTracks: new Set(), + selectedTracks: [], }; case "INVERT_SELECTION": return { ...state, - selectedTracks: new Set( - state.trackOrder.filter((id) => !state.selectedTracks.has(id)) + selectedTracks: state.trackOrder.filter( + (id) => !state.selectedTracks.includes(id) ), }; case "REMOVE_ALL_TRACKS": @@ -82,7 +79,7 @@ const playlist = ( ...state, trackOrder: [], currentTrack: null, - selectedTracks: new Set(), + selectedTracks: [], lastSelectedIndex: null, }; case "REMOVE_TRACKS": @@ -95,8 +92,8 @@ const playlist = ( (trackId) => !actionIds.has(trackId) ), currentTrack: actionIds.has(Number(currentTrack)) ? null : currentTrack, - selectedTracks: new Set( - Array.from(state.selectedTracks).filter((id) => actionIds.has(id)) + selectedTracks: Array.from(state.selectedTracks).filter((id) => + actionIds.has(id) ), // TODO: This could probably be made to work, but we clear it just to be safe. lastSelectedIndex: null, @@ -142,7 +139,7 @@ const playlist = ( ...state, trackOrder: moveSelected( state.trackOrder, - (i) => state.selectedTracks.has(state.trackOrder[i]), + (i) => state.selectedTracks.includes(state.trackOrder[i]), (action as any).offset ), // TODO: This could probably be made to work, but we clear it just to be safe. diff --git a/packages/webamp/js/selectors.ts b/packages/webamp/js/selectors.ts index 2f776d01..b97b42a3 100644 --- a/packages/webamp/js/selectors.ts +++ b/packages/webamp/js/selectors.ts @@ -116,13 +116,20 @@ const getOrderedTrackObjects = createSelector( (tracks, trackOrder): PlaylistTrack[] => trackOrder.map((id) => tracks[id]) ); -export const getSelectedTrackIds = (state: AppState): Set => { +export const getSelectedTrackIds = (state: AppState): Array => { return state.playlist.selectedTracks; }; +export const getSelectedTrackIdsSet = createSelector( + getSelectedTrackIds, + (selectedTrackArray): Set => { + return new Set(selectedTrackArray); + } +); + export const getSelectedTrackObjects = createSelector( getOrderedTrackObjects, - getSelectedTrackIds, + getSelectedTrackIdsSet, (tracks, selectedIds) => tracks.filter((track) => selectedIds.has(track.id)) );