refactor(media-store): replace mediaEvents with stateOwnersUpdateHandlers pattern

- Replace simple mediaEvents arrays with stateOwnersUpdateHandlers functions
- Add cleanup function returns to prevent memory leaks
- Update factory to use new event handler pattern with proper teardown
- Maintain backward compatibility with all existing functionality
- Enable support for multiple state owners (media, document, etc.)
- All existing controls (play/pause, volume, seek) tested and working

This refactor provides the foundation for cross-platform fullscreen support
and other multi-state-owner scenarios while following Media Chrome patterns.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Christian Pillsbury
2025-09-12 08:06:08 -07:00
committed by Christian Pillsbury
co-authored by Claude
parent 517bb30678
commit da94272f63
4 changed files with 111 additions and 34 deletions
+30 -27
View File
@@ -18,13 +18,13 @@ export type FacadeGetter<T, D = T> = (
export type FacadeSetter<T> = (value: T, stateOwners: StateOwners) => void;
export type StateOwnerUpdateHandler<T> = (
handler: (value: T) => void,
handler: (value?: T) => void,
stateOwners: StateOwners,
) => void;
) => (() => void) | void;
export type ReadonlyFacadeProp<T, D = T> = {
get: FacadeGetter<T, D>;
mediaEvents?: string[];
stateOwnersUpdateHandlers?: StateOwnerUpdateHandler<T>[];
};
export type FacadeProp<T, S = T, D = T> = ReadonlyFacadeProp<T, D> & {
@@ -56,7 +56,7 @@ export function createMediaStore({
}) {
const stateOwners: StateOwners = {};
const store = map<any>({});
const stateUpdateHandlers: Record<string, () => void> = {};
const stateUpdateHandlerCleanups: Record<string, (() => void)[]> = {};
const keys = Object.keys(stateMediator);
function updateStateOwners(nextStateOwners: any) {
@@ -64,34 +64,37 @@ export function createMediaStore({
return;
}
let media = stateOwners.media;
if (media) {
for (const { mediaEvents = [] } of Object.values(stateMediator)) {
for (const mediaEvent of mediaEvents) {
media.removeEventListener(
mediaEvent,
stateUpdateHandlers[mediaEvent],
);
delete stateUpdateHandlers[mediaEvent];
}
}
}
// Clean up existing handlers
Object.entries(stateUpdateHandlerCleanups).forEach(([stateName, cleanups]) => {
cleanups.forEach(cleanup => cleanup?.());
stateUpdateHandlerCleanups[stateName] = [];
});
Object.assign(stateOwners, nextStateOwners);
media = stateOwners.media;
store.set(getInitialState(stateMediator, stateOwners));
if (media) {
for (const [stateName, stateObject] of Object.entries(stateMediator)) {
const { get, mediaEvents = [] } = stateObject;
for (const mediaEvent of mediaEvents) {
stateUpdateHandlers[mediaEvent] = () =>
store.setKey(stateName, get(stateOwners));
media.addEventListener(mediaEvent, stateUpdateHandlers[mediaEvent]);
}
// Set up new handlers
Object.entries(stateMediator).forEach(([stateName, stateObject]) => {
const { get, stateOwnersUpdateHandlers = [] } = stateObject;
if (!stateUpdateHandlerCleanups[stateName]) {
stateUpdateHandlerCleanups[stateName] = [];
}
}
// Create handler that updates the store
const updateHandler = (value?: any) => {
const nextValue = value !== undefined ? value : get(stateOwners);
store.setKey(stateName, nextValue);
};
// Execute each stateOwnersUpdateHandler
stateOwnersUpdateHandlers.forEach(setupHandler => {
const cleanup = setupHandler(updateHandler, stateOwners);
if (typeof cleanup === 'function') {
stateUpdateHandlerCleanups[stateName]?.push(cleanup);
}
});
});
}
return {
@@ -12,7 +12,17 @@ export const audible = {
media.volume = 0.25;
}
},
mediaEvents: ['volumechange'],
stateOwnersUpdateHandlers: [
(handler: (value?: boolean) => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
media.addEventListener('volumechange', eventHandler);
return () => media.removeEventListener('volumechange', eventHandler);
}
],
actions: {
/** @TODO Refactor me to play more nicely with side effects that don't/can't correlate with set() API or aren't simple 1:1 with getter vs. setter (CJP) */
muterequest: () => true,
@@ -34,7 +44,17 @@ export const audible = {
media.mute = false;
}
},
mediaEvents: ['volumechange'],
stateOwnersUpdateHandlers: [
(handler: (value?: number) => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
media.addEventListener('volumechange', eventHandler);
return () => media.removeEventListener('volumechange', eventHandler);
}
],
actions: {
/** @TODO Refactor me to play more nicely with side effects that don't/can't correlate with set() API (CJP) */
volumerequest: (
@@ -52,6 +72,16 @@ export const audible = {
if (media.volume < 0.75) return 'medium';
return 'high';
},
mediaEvents: ['volumechange'],
stateOwnersUpdateHandlers: [
(handler: (value?: 'high' | 'medium' | 'low' | 'off') => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
media.addEventListener('volumechange', eventHandler);
return () => media.removeEventListener('volumechange', eventHandler);
}
],
},
};
@@ -8,7 +8,18 @@ export const playable = {
const { media } = stateOwners;
media?.[value ? 'pause' : 'play']();
},
mediaEvents: ['play', 'playing', 'pause', 'emptied'],
stateOwnersUpdateHandlers: [
(handler: (value?: boolean) => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
const events = ['play', 'playing', 'pause', 'emptied'];
events.forEach(event => media.addEventListener(event, eventHandler));
return () => events.forEach(event => media.removeEventListener(event, eventHandler));
}
],
actions: {
/** @TODO Refactor me to play more nicely with side effects that don't/can't correlate with set() API (CJP) */
playrequest: () => false,
@@ -15,7 +15,18 @@ export const temporal = {
if (!media || !isValidNumber(value)) return;
media.currentTime = value;
},
mediaEvents: ['timeupdate', 'loadedmetadata'],
stateOwnersUpdateHandlers: [
(handler: (value?: number) => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
const events = ['timeupdate', 'loadedmetadata'];
events.forEach(event => media.addEventListener(event, eventHandler));
return () => events.forEach(event => media.removeEventListener(event, eventHandler));
}
],
actions: {
/** @TODO Support more sophisticated seeking patterns like seek-to-live, relative seeking, etc. (CJP) */
seekrequest: (
@@ -39,7 +50,18 @@ export const temporal = {
return media.duration;
},
mediaEvents: ['loadedmetadata', 'durationchange', 'emptied'],
stateOwnersUpdateHandlers: [
(handler: (value?: number) => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
const events = ['loadedmetadata', 'durationchange', 'emptied'];
events.forEach(event => media.addEventListener(event, eventHandler));
return () => events.forEach(event => media.removeEventListener(event, eventHandler));
}
],
},
seekable: {
@@ -56,6 +78,17 @@ export const temporal = {
return [Number(start.toFixed(3)), Number(end.toFixed(3))];
},
mediaEvents: ['loadedmetadata', 'emptied', 'progress', 'seekablechange'],
stateOwnersUpdateHandlers: [
(handler: (value?: [number, number] | undefined) => void, stateOwners: any) => {
const { media } = stateOwners;
if (!media) return;
const eventHandler = () => handler();
const events = ['loadedmetadata', 'emptied', 'progress', 'seekablechange'];
events.forEach(event => media.addEventListener(event, eventHandler));
return () => events.forEach(event => media.removeEventListener(event, eventHandler));
}
],
},
};