feat: add shared memo client for react embeds#6
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
3 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/memos-embed-react/src/index.tsx">
<violation number="1" location="packages/memos-embed-react/src/index.tsx:109">
P1: `useContext` is called conditionally in `useMemoClient`, which can break hook ordering across renders.</violation>
</file>
<file name="packages/memos-embed/src/api.ts">
<violation number="1" location="packages/memos-embed/src/api.ts:158">
P1: Shared user caching is keyed only by user ID, so one `createMemoClient` used across multiple `baseUrl` values can return creator data from the wrong instance.</violation>
<violation number="2" location="packages/memos-embed/src/api.ts:315">
P2: `createMemoClient().fetchMemo` drops the `signal` option, so abort/cancellation passed by callers is silently ignored.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ); | ||
|
|
||
| export const useMemoClient = (client?: MemoClient) => | ||
| client ?? useContext(MemoClientContext); |
There was a problem hiding this comment.
P1: useContext is called conditionally in useMemoClient, which can break hook ordering across renders.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/memos-embed-react/src/index.tsx, line 109:
<comment>`useContext` is called conditionally in `useMemoClient`, which can break hook ordering across renders.</comment>
<file context>
@@ -86,6 +91,23 @@ export type MemoEmbedListProps =
+);
+
+export const useMemoClient = (client?: MemoClient) =>
+ client ?? useContext(MemoClientContext);
+
const renderState = ({
</file context>
| const normalizedBaseUrl = normalizeBaseUrl(baseUrl); | ||
| const id = userId.includes("/") ? getIdFromName(userId) : userId; | ||
| const cached = context.userCache.get(id); | ||
| const cached = userCache.get(id); |
There was a problem hiding this comment.
P1: Shared user caching is keyed only by user ID, so one createMemoClient used across multiple baseUrl values can return creator data from the wrong instance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/memos-embed/src/api.ts, line 158:
<comment>Shared user caching is keyed only by user ID, so one `createMemoClient` used across multiple `baseUrl` values can return creator data from the wrong instance.</comment>
<file context>
@@ -114,37 +133,38 @@ const normalizeMemo = (
+ const normalizedBaseUrl = normalizeBaseUrl(baseUrl);
const id = userId.includes("/") ? getIdFromName(userId) : userId;
- const cached = context.userCache.get(id);
+ const cached = userCache.get(id);
if (cached) {
return cached;
</file context>
Comment on lines
+315
to
+328
| const clientFetchMemo: MemoClient["fetchMemo"] = async ({ | ||
| baseUrl, | ||
| memoId, | ||
| includeCreator = true, | ||
| fetcher, | ||
| }) => | ||
| fetchMemoWithCache({ | ||
| baseUrl, | ||
| memoId, | ||
| includeCreator, | ||
| fetcher: fetcher ?? config.fetcher ?? fetch, | ||
| userCache, | ||
| memoCache, | ||
| }); |
There was a problem hiding this comment.
P2: createMemoClient().fetchMemo drops the signal option, so abort/cancellation passed by callers is silently ignored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/memos-embed/src/api.ts, line 315:
<comment>`createMemoClient().fetchMemo` drops the `signal` option, so abort/cancellation passed by callers is silently ignored.</comment>
<file context>
@@ -191,33 +225,144 @@ const fetchMemoWithContext = async ({
+ const memoCache: MemoCache = new Map();
+ const userCache: UserCache = new Map();
+
+ const clientFetchMemo: MemoClient["fetchMemo"] = async ({
+ baseUrl,
+ memoId,
</file context>
Suggested change
| const clientFetchMemo: MemoClient["fetchMemo"] = async ({ | |
| baseUrl, | |
| memoId, | |
| includeCreator = true, | |
| fetcher, | |
| }) => | |
| fetchMemoWithCache({ | |
| baseUrl, | |
| memoId, | |
| includeCreator, | |
| fetcher: fetcher ?? config.fetcher ?? fetch, | |
| userCache, | |
| memoCache, | |
| }); | |
| const clientFetchMemo: MemoClient["fetchMemo"] = async ({ | |
| baseUrl, | |
| memoId, | |
| includeCreator = true, | |
| fetcher, | |
| signal, | |
| }) => | |
| fetchMemoWithCache({ | |
| baseUrl, | |
| memoId, | |
| includeCreator, | |
| fetcher: fetcher ?? config.fetcher ?? fetch, | |
| signal, | |
| userCache, | |
| memoCache, | |
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createMemoClient()in core for cross-embed memo and creator request dedupingMemoClientProvideranduseMemoClient()in@memos-embed/react, wiring bothMemoEmbedandMemoEmbedListinto shared client usageTesting
Summary by cubic
Adds a shared memo client to
memos-embedwith a React provider/hook so multiple embeds reuse memo/creator requests and avoid duplicate fetches. Docs and tests were updated to cover the new workflow.memos-embed:createMemoClient()with in-memory caching,fetchMemo/fetchMemos,primeMemo/primeMemos, andclear().@memos-embed/react:MemoClientProvideranduseMemoClient();MemoEmbedandMemoEmbedListaccept an optionalclientprop and use the shared client when available (prefetched data is primed into the client; without a client, existing per-component fetch/abort behavior remains).Written for commit 3793df7. Summary will update on new commits.