Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quick-rings-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@livekit/components-react': patch
---

Fix useSession return value stability
88 changes: 79 additions & 9 deletions packages/react/src/hooks/useSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,84 @@ type UseSessionCommonOptions = {
type UseSessionConfigurableOptions = UseSessionCommonOptions & TokenSourceFetchOptions;
type UseSessionFixedOptions = UseSessionCommonOptions;

/**
* Given two TokenSourceFetchOptions values, check to see if they are deep equal.
*
* FIXME: swap this for an import from livekit-client once
* https://github.com/livekit/client-sdk-js/pull/1733 is merged and published!
* */
function areTokenSourceFetchOptionsEqual(a: TokenSourceFetchOptions, b: TokenSourceFetchOptions) {
const allKeysSet = new Set([...Object.keys(a), ...Object.keys(b)]) as Set<
keyof TokenSourceFetchOptions
>;

for (const key of allKeysSet) {
switch (key) {
case 'roomName':
case 'participantName':
case 'participantIdentity':
case 'participantMetadata':
case 'participantAttributes':
case 'agentName':
case 'agentMetadata':
if (a[key] !== b[key]) {
return false;
}
break;
default:
// ref: https://stackoverflow.com/a/58009992
const exhaustiveCheckedKey: never = key;
throw new Error(`Options key ${exhaustiveCheckedKey} not being checked for equality!`);
}
}

return true;
}
Comment on lines +157 to +189
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chore: This has been copied from here: livekit/client-sdk-js#1733. I did this so I could test the change but IMO this copied code shouldn't be merged.

So, before this should be merged, get the above linked pull request deployed into a new version on npm, then delete this code and update it to be an import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I forgot to do this before merging 🤦 . I will do this as part of the follow up change.


/** Internal hook used by useSession to manage creating a function that properly invokes
* tokenSource.fetch(...) with any fetch options */
function useSessionTokenSourceFetch(
tokenSource: TokenSourceConfigurable | TokenSourceFixed,
unstableRestOptions: Exclude<UseSessionConfigurableOptions, keyof UseSessionCommonOptions>,
) {
const isConfigurable = tokenSource instanceof TokenSourceConfigurable;

const memoizedTokenFetchOptionsRef = React.useRef<TokenSourceFetchOptions | null>(
isConfigurable ? unstableRestOptions : null,
);

React.useEffect(() => {
if (!isConfigurable) {
memoizedTokenFetchOptionsRef.current = null;
return;
}

if (
memoizedTokenFetchOptionsRef.current !== null &&
areTokenSourceFetchOptionsEqual(memoizedTokenFetchOptionsRef.current, unstableRestOptions)
) {
return;
}

memoizedTokenFetchOptionsRef.current = unstableRestOptions;
}, [isConfigurable, unstableRestOptions]);

const tokenSourceFetch = React.useCallback(async () => {
if (isConfigurable) {
if (!memoizedTokenFetchOptionsRef.current) {
throw new Error(
`AgentSession - memoized token fetch options are not set, but the passed tokenSource was an instance of TokenSourceConfigurable. If you are seeing this please make a new GitHub issue!`,
);
}
return tokenSource.fetch(memoizedTokenFetchOptionsRef.current);
} else {
return tokenSource.fetch();
}
}, [isConfigurable, tokenSource]);

return tokenSourceFetch;
}

/**
* A Session represents a manages connection to a Room which can contain Agents.
* @public
Expand Down Expand Up @@ -427,15 +505,7 @@ export function useSession(
),
);

const tokenSourceFetch = React.useCallback(async () => {
const isConfigurable = tokenSource instanceof TokenSourceConfigurable;
if (isConfigurable) {
const tokenFetchOptions = restOptions as UseSessionConfigurableOptions;
return tokenSource.fetch(tokenFetchOptions);
} else {
return tokenSource.fetch();
}
}, [tokenSource, restOptions]);
const tokenSourceFetch = useSessionTokenSourceFetch(tokenSource, restOptions);

const start = React.useCallback(
async (connectOptions: SessionConnectOptions = {}) => {
Expand Down