fix: address community refactor follow-up bugs#24
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR renames the domain concept "subplebbit" to "community" across docs, hooks, stores, utils, mocks, and tests; removes the old subplebbits hooks file; and adds new community-focused stores and hooks (communities store and communities-pages store) with caching, event subscriptions, and extensive test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useCommunity as useCommunity Hook
participant CommunitiesStore as CommunitiesStore
participant LocalForage as LocalForage Cache
participant PlebbitJS as PlebbitJS API
Component->>useCommunity: mount(communityAddress)
useCommunity->>CommunitiesStore: check in-memory cache
alt cached
CommunitiesStore->>Component: return cached community
else not cached
useCommunity->>LocalForage: load cached community
alt cache hit & fresh
LocalForage->>CommunitiesStore: hydrate store
CommunitiesStore->>Component: return cached community
else cache miss or stale
useCommunity->>PlebbitJS: getCommunity(address)
PlebbitJS->>CommunitiesStore: emit fetched community
CommunitiesStore->>LocalForage: persist community
CommunitiesStore->>Component: return fresh community
end
end
PlebbitJS-->>CommunitiesStore: 'update' event (community)
CommunitiesStore->>Component: notify subscribers of update
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/plebbit-js/plebbit-js-mock-content.donttest.ts (1)
280-285:⚠️ Potential issue | 🟡 MinorBind the second hook render to its own
waitFor.Line 285 still uses the original
waitFor, which is tied torendered. AfterresetStores(), that can make the DB-cache check wait on stale hook state instead ofrendered2.🧪 Suggested fix
await testUtils.resetStores(); const rendered2 = renderHook<any, any>((communityAddress) => useCommunity({ communityAddress }), ); + const waitFor2 = testUtils.createWaitFor(rendered2, { timeout }); rendered2.rerender("anything2.eth"); - await waitFor(() => typeof rendered2.result.current.updatedAt === "number"); + await waitFor2(() => typeof rendered2.result.current.updatedAt === "number");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/plebbit-js/plebbit-js-mock-content.donttest.ts` around lines 280 - 285, The second hook render (rendered2 from renderHook/useCommunity) is still awaiting using the original waitFor tied to the first render; after resetStores() this can wait on stale state. Change the test to use a waitFor bound to the second render (e.g., obtain the waitFor from rendered2 or call rendered2.waitFor) and await that for the updatedAt check so the DB-cache check waits on rendered2.result.current rather than the original render's state.src/hooks/comments.ts (1)
240-251:⚠️ Potential issue | 🟡 MinorGuard against stale validation results.
If
commentoraccount?.plebbitchanges while an earliercommentIsValid()call is still in flight, the older promise can still win and overwritevalidatedfor the newer input.Possible fix
useEffect(() => { + let cancelled = false; + if (!comment || !account?.plebbit) { setValidated(undefined); return; } // don't automatically block community because what community it comes from // a malicious community could try to block other communities, etc const blockCommunity = false; commentIsValid(comment, { validateReplies, blockCommunity }, account.plebbit).then( - (validated) => setValidated(validated), + (validated) => { + if (!cancelled) { + setValidated(validated); + } + }, ); + + return () => { + cancelled = true; + }; }, [comment, validateReplies, account?.plebbit]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/comments.ts` around lines 240 - 251, The current useEffect calls commentIsValid asynchronously and sets state via setValidated, but does not guard against stale responses; if comment or account?.plebbit changes while a prior promise is in flight the older result can overwrite the newer one. Fix by introducing a cancellation/sequence guard inside the effect (e.g. a local let cancelled=false or seq id captured by the promise) and only call setValidated when the call is still current (not cancelled and sequence matches); ensure you clear the flag or increment the sequence in the cleanup function so commentIsValid() results for prior inputs are ignored. Reference the useEffect block, commentIsValid(), setValidated, comment, account?.plebbit and validateReplies when applying the guard.src/hooks/feeds/feeds.test.ts (1)
208-230:⚠️ Potential issue | 🟠 MajorWrap clock/prototype patches in
try/finally.These cases replace
Date.nowand severalPlebbit/Community/Pagesprototype methods, but restoration is only on the success path. Any failed assertion will leak the patch into later feed tests and make the suite flaky.Also applies to: 257-291, 297-348, 499-519, 604-631, 1117-1137, 1276-1289, 1300-1352, 1360-1371, 1561-1586, 1590-1630, 1634-1697, 1755-1840, 1844-1876, 2292-2327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/feeds/feeds.test.ts` around lines 208 - 230, The test patches (e.g., Date.now and Community.prototype.update) must be wrapped in a try/finally so the original implementations are always restored on failure; change the block that replaces Date.now and Community.prototype.update to store originals (e.g., const DateNow = Date.now; const update = Community.prototype.update), perform the test logic inside try { ... } and put Date.now = DateNow; Community.prototype.update = update; in finally { ... }. Apply the same try/finally pattern to every test that mutates globals or prototypes (Plebbit.prototype, Pages.prototype, Community.prototype, Date.now, etc.) so restorations cannot be skipped by thrown assertions.docs/schema.md (1)
52-62:⚠️ Potential issue | 🟡 MinorThis
filtercontract no longer matches the hook API.
useAccountComments()anduseAccountVotes()currently assert thatoptions.filteris a function insrc/hooks/accounts/accounts.ts. Documenting it as an object withcommunityAddresses,postCids, etc. will keep producing broken examples after the rename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/schema.md` around lines 52 - 62, The docs show UseAccountsCommentsOptions.filter as an object but the hooks useAccountComments and useAccountVotes in src/hooks/accounts/accounts.ts expect options.filter to be a predicate function; update the schema docs so UseAccountCommentsFilter is defined as a function type (a predicate that accepts a comment/vote item and returns boolean) and adjust UseAccountsCommentsOptions to indicate filter: UseAccountCommentsFilter (function) or optional, matching the runtime assertion in useAccountComments/useAccountVotes; reference the function name UseAccountCommentsFilter and the hooks useAccountComments/useAccountVotes to locate and align the contract.README.md (1)
952-985:⚠️ Potential issue | 🟡 MinorThese block/unblock snippets contain copy-paste errors preventing execution.
Line 952:
const address: 'community-address.eth'has invalid syntax (missing=operator). Should beconst address = 'community-address.eth'.CID example: logs undefined variable
cidwithout declaring it. The code passescid: "Qm..."touseBlock()but never declaresconst cid = "Qm..."before using it in the log statements.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 952 - 985, The examples using useBlock contain simple declaration bugs: replace the invalid declaration const address: 'community-address.eth' with a proper assignment (declare const address = 'community-address.eth') and ensure the CID example declares the same variable passed to useBlock (e.g., const cid = "Qm..." before calling useBlock({ cid })) so the subsequent logs referencing address or cid and calls to block() / unblock() use defined variables; update the log templates to reference the declared symbols (address or cid) accordingly.
🟡 Minor comments (12)
src/lib/chain/chain.test.ts-70-79 (1)
70-79:⚠️ Potential issue | 🟡 MinorAssert the canonical wallet message string directly.
This test only proves the string survives
JSON.parse/JSON.stringifyround-tripping. It would still pass ifgetWalletMessageToSignemitted the fields in the wrong order, which is exactly the regressiondist/lib/validator.js:173-177warns against because signature verification depends on a fixed property order.Proposed fix
test("getWalletMessageToSign", () => { - const string = getWalletMessageToSign(authorAddress, walletTimestamp); - const json = JSON.parse(string); - expect(json.domainSeparator).toBe("plebbit-author-wallet"); - expect(json.authorAddress).toBe(authorAddress); - expect(json.authorAddress).not.toBe(undefined); - expect(json.timestamp).toBe(walletTimestamp); - expect(json.timestamp).not.toBe(undefined); - expect(string).toBe(JSON.stringify(json)); + expect(getWalletMessageToSign(authorAddress, walletTimestamp)).toBe( + `{"domainSeparator":"plebbit-author-wallet","authorAddress":"${authorAddress}","timestamp":${walletTimestamp}}`, + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chain/chain.test.ts` around lines 70 - 79, The test for getWalletMessageToSign must assert the exact canonical JSON string (with fixed property order) instead of round-tripping via JSON.parse/JSON.stringify; update the test in chain.test.ts to build the expected canonical string (e.g., JSON.stringify({ domainSeparator: "plebbit-author-wallet", authorAddress: authorAddress, timestamp: walletTimestamp }) with keys in the correct order) and assert expect(string).toBe(expectedString) so getWalletMessageToSign returns the exact ordered string required for signature verification.src/lib/chain/chain.test.ts-62-68 (1)
62-68:⚠️ Potential issue | 🟡 MinorRestore
Date.nowon the failure path too.Each block mutates a global and restores it only after the awaited call succeeds. If wallet generation throws, the mocked clock leaks into later tests and turns one failure into a suite-wide red herring.
Safer pattern
beforeAll(async () => { privateKey = await getEthPrivateKeyFromPlebbitPrivateKey(plebbitPrivateKey); const dateNow = Date.now; Date.now = () => walletTimestamp * 1000; - wallet = await getEthWalletFromPlebbitPrivateKey(plebbitPrivateKey, authorAddress); - Date.now = dateNow; + try { + wallet = await getEthWalletFromPlebbitPrivateKey(plebbitPrivateKey, authorAddress); + } finally { + Date.now = dateNow; + } });Also applies to lines 157-163 and 177-182.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chain/chain.test.ts` around lines 62 - 68, The test mutates Date.now before awaiting getEthWalletFromPlebbitPrivateKey (and similarly in other beforeAll blocks) but only restores it after the await, so if the await throws the mock leaks; wrap the Date.now override and the awaited call in a try/finally (or otherwise ensure restoration) so Date.now is always set back even on failure—apply this fix around the getEthWalletFromPlebbitPrivateKey/getEthPrivateKeyFromPlebbitPrivateKey blocks referenced in the beforeAll hooks (also update the blocks at the other two locations mentioned).src/lib/community-address.ts-9-10 (1)
9-10:⚠️ Potential issue | 🟡 MinorCanonical community addresses are still case-sensitive for
.bsoinputs.
getCanonicalCommunityAddress("Music.BSO")returns"Music.BSO", while the equivalent lowercase input returns"music.bso".src/hooks/accounts/accounts.tsuses this helper as theaccountCommunitiesmap key, so mixed-case aliases can still produce unstable/split entries.🔧 Suggested fix
-export const getCanonicalCommunityAddress = (address: string) => - address.toLowerCase().endsWith(".eth") ? address.slice(0, -4) + ".bso" : address; +export const getCanonicalCommunityAddress = (address: string) => { + const lower = address.toLowerCase(); + if (lower.endsWith(".eth")) { + return lower.slice(0, -4) + ".bso"; + } + if (lower.endsWith(".bso")) { + return lower; + } + return address; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/community-address.ts` around lines 9 - 10, getCanonicalCommunityAddress currently only lowercases when checking for ".eth" but returns the original casing for ".bso" inputs, causing inconsistent keys; change the function to normalize the whole input to lowercase first (e.g., const normalized = address.toLowerCase()) and then apply the existing logic so that getCanonicalCommunityAddress always returns a lowercase address (if normalized.endsWith(".eth") return normalized.slice(0, -4) + ".bso", else return normalized). Ensure you update the function name getCanonicalCommunityAddress accordingly so callers (like accountCommunities in src/hooks/accounts/accounts.ts) receive stable lowercase keys.src/lib/validator.ts-394-409 (1)
394-409:⚠️ Potential issue | 🟡 MinorInconsistent error messages in
validateUseCommunitiesArguments.The error messages at lines 397 and 407 reference
useCommunity(singular) but should referenceuseCommunities(plural) to match the function name.🐛 Proposed fix
const validateUseCommunitiesArguments = (communityAddresses: any, account: any) => { assert( Array.isArray(communityAddresses), - `useCommunity communityAddresses '${toString(communityAddresses)}' not an array`, + `useCommunities communityAddresses '${toString(communityAddresses)}' not an array`, ); for (const communityAddress of communityAddresses) { assert( typeof communityAddress === "string", `useCommunities communityAddresses '${toString(communityAddresses)}' communityAddress '${toString(communityAddress)}' not a string`, ); } assert( account?.plebbit && typeof account?.plebbit === "object", - `useCommunity account.plebbit '${account?.plebbit}' not an object`, + `useCommunities account.plebbit '${account?.plebbit}' not an object`, ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/validator.ts` around lines 394 - 409, The error strings in validateUseCommunitiesArguments mistakenly reference "useCommunity" (singular); update those messages to "useCommunities" (plural) so they match the function name and intent—specifically change the first assert message that mentions `useCommunity communityAddresses` and the final account assert that mentions `useCommunity account.plebbit` to use `useCommunities` instead; keep the existing toString(...) context and overall message format unchanged.src/hooks/feeds/feeds.ts-74-75 (1)
74-75:⚠️ Potential issue | 🟡 MinorGuard empty
communityAddressesinloadMoreandreset.Line 74 treats an empty community list as uninitialized, but Line 106 and Line 119 only check the array object. Because
useUniqueSorted()returns[], both methods still run when no feed was added.Suggested fix
const loadMore = async () => { try { - if (!uniqueCommunityAddresses || !account) { + if (!uniqueCommunityAddresses?.length || !account) { throw Error("useFeed cannot load more feed not initalized yet"); } incrementFeedPageNumber(feedName); } catch (e: any) { // wait 100 ms so infinite scroll doesn't spam this function @@ const reset = async () => { try { - if (!uniqueCommunityAddresses || !account) { + if (!uniqueCommunityAddresses?.length || !account) { throw Error("useFeed cannot reset feed not initalized yet"); } await resetFeed(feedName); } catch (e: any) { // wait 100 ms so infinite scroll doesn't spam this functionAlso applies to: 100-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/feeds/feeds.ts` around lines 74 - 75, The guard at the top uses if (!uniqueCommunityAddresses?.length || !account) return;, but the loadMore and reset functions only check the array object and run when uniqueCommunityAddresses is [] (empty); update both loadMore and reset to explicitly check for an empty array (e.g., !uniqueCommunityAddresses?.length) before proceeding. Locate the loadMore and reset functions in the same file (references: loadMore, reset, uniqueCommunityAddresses, useUniqueSorted) and add the same empty-length guard (and account check where applicable) to prevent execution when no communities exist.docs/algorithms.md-34-45 (1)
34-45:⚠️ Potential issue | 🟡 MinorThese store signatures no longer match the code.
addCommunityToStoreis exercised as(communityAddress, account)insrc/stores/communities/communities-store.test.ts, andaddNextCommunityPageToStoreis exercised as(community, sortType, account)insrc/stores/communities-pages/communities-pages-store.test.ts. Documenting the old one-argument forms will mislead anyone wiring these stores directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/algorithms.md` around lines 34 - 45, Update the documented store signatures to match the actual usage in tests: change addCommunityToStore to accept (communityAddress, account) and change addNextCommunityPageToStore to accept (community, sortType, account); ensure the entries for communitiesStore and communitiesPagesStore reference the correct parameter names and semantics used by the functions addCommunityToStore and addNextCommunityPageToStore so the docs match the implementations and tests.docs/algorithms.md-123-130 (1)
123-130:⚠️ Potential issue | 🟡 MinorThe replies-store block is still describing feed/community state.
For replies,
bufferedPostsCountsshould be keyed off the reply feed/comment, andupdateFeeds()is driven bycomment.replies.pagesplusrepliesPagesStore, notcommunityAddressorcommunitiesPagesStore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/algorithms.md` around lines 123 - 130, The replies-store schema and update logic are incorrectly tied to community feed state: change bufferedPostsCounts to use reply-specific keys (e.g., key by replyFeedId or commentId + sortType instead of communityAddress) and update updateFeeds() to recalculate using comment.replies.pages and repliesPagesStore rather than communities.post.pages and communitiesPagesStore; locate references to bufferedPostsCounts and updateFeeds() and replace community-based key construction/reads with reply-based key construction and read from repliesPagesStore/comment.replies.pages so replies pagination is driven by the correct stores.docs/clients.md-85-93 (1)
85-93:⚠️ Potential issue | 🟡 MinorAdd
errorandcommunity.errorto the dependency arrays in these threeuseMemoexamples.The memo bodies read
errororcommunity.error, but the dependency arrays only include the state flags. React'sexhaustive-depsrule requires all reactive values read inside the callback to be listed in dependencies. Missing these will cause stale error text to render and trigger linting errors.Also applies to lines 110–118 and 248–256.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/clients.md` around lines 85 - 93, The useMemo callbacks (e.g., the one producing errorString) read reactive values "error" and "community.error" but only list state flags in their dependency arrays, causing stale values and lint failures; update the three useMemo calls (the errorString useMemo and the two other similar useMemo blocks referenced around lines 110–118 and 248–256) to include the exact reactive variables they read—add "error" and "community?.error" (or "community.error" if non-nullable) to each dependency array alongside the existing state entries so the memo re-computes when those error values change.src/hooks/actions/actions.ts-681-685 (1)
681-685:⚠️ Potential issue | 🟡 Minor
useCreateCommunity()should be ready with no options.
UseCreateCommunityOptionsis optional, but this keeps the hook in"initializing"until the caller passes an object. Components that gate onstate === "ready"will treat the default create flow as unavailable.Possible fix
- if (accountId && options) { + if (accountId) { initialState = "ready"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/actions/actions.ts` around lines 681 - 685, The hook currently sets initialState = "ready" only when both accountId and options exist, which blocks ready state when options are omitted; update the logic in useCreateCommunity (the initialState assignment) to treat UseCreateCommunityOptions as optional by making initialState "ready" when accountId is present (e.g., change the condition from `if (accountId && options)` to `if (accountId)`) or give the options parameter a default value (e.g., default to `{}`) so callers who omit options still get state === "ready".README.md-1245-1248 (1)
1245-1248:⚠️ Potential issue | 🟡 Minor
getShortCidandgetShortAddressare swapped.Line 1247 uses
getShortAddress()on a CID, and line 1248 usesgetShortCid()on an address. These should be reversed:getShortCid()expects a CID string andgetShortAddress()expects an address string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1245 - 1248, The calls to the helper functions are swapped: swap the two usages so getShortCid is called with the CID and getShortAddress is called with the address; specifically, replace the usage of getShortAddress(comment.parentCid) with getShortCid(comment.parentCid) and replace getShortCid(address) with getShortAddress(address) to match the expected parameter types for getShortCid and getShortAddress.README.md-1038-1068 (1)
1038-1068:⚠️ Potential issue | 🟡 MinorThe community-edit walkthrough references undeclared values.
The first example calls
publishCommunityEdit()without showing the precedingusePublishCommunityEdit()hook initialization. Additionally, the verification block usesstate,error, andchainProvidervariables that are not declared; onlyresolvedAddressis destructured fromuseResolvedCommunityAddress(). The section is not copy-pasteable as written.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1038 - 1068, The examples call publishCommunityEdit without showing the required hook and reference undeclared variables in the verification snippet; fix by adding the missing hook initialization before calling publishCommunityEdit (e.g., create editCommunityOptions and call const {publishCommunityEdit} = usePublishCommunityEdit(editCommunityOptions) prior to await publishCommunityEdit()), ensure the first ENS example uses a unique variable name or reuses the same editCommunityOptions consistently, and update the verification example to destructure all returned values from useResolvedCommunityAddress (e.g., const {resolvedAddress, state, error, chainProvider} = useResolvedCommunityAddress({communityAddress: 'your-community-address.eth', cache: false})) so state/error/chainProvider are declared before they are used.src/hooks/states.ts-223-230 (1)
223-230:⚠️ Potential issue | 🟡 MinorUse a block-bodied
forEachcallback here.Biome's
useIterableCallbackReturnrule flags the expression-bodied callback at lines 228-230 because it implicitly returns the result ofSet.add(). Wrap the callback body in braces to eliminate the unused return value:Current code
pagesClientsUrls[pagesState].forEach((clientUrl: string) => states[pagesState].clientUrls.add(clientUrl), );Fixed code
pagesClientsUrls[pagesState].forEach((clientUrl: string) => { states[pagesState].clientUrls.add(clientUrl); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/states.ts` around lines 223 - 230, The forEach callback on pagesClientsUrls[pagesState] currently uses an expression-bodied arrow that indirectly returns Set.add(), triggering the useIterableCallbackReturn rule; change the callback in the forEach on pagesClientsUrls[pagesState] to a block-bodied arrow (wrap body in { ... }) and call states[pagesState].clientUrls.add(clientUrl); inside the block so the callback does not return a value—keep the surrounding loop and use of community.address and states as-is.
🧹 Nitpick comments (9)
src/lib/localforage-lru/localforage-lru.test.ts (1)
59-59: Optional: Consider adding a cleanup/reset method to the module.The tests directly manipulate the exported
instancesobject to simulate reinstantiation. While this works andinstancesis part of the public API, it couples the tests to implementation details. A dedicatedresetInstance(name)ordestroyInstance(name)method would provide a cleaner testing interface and better encapsulation.Also applies to: 76-76, 96-96, 103-103, 120-120, 128-128, 136-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/localforage-lru/localforage-lru.test.ts` at line 59, Tests currently mutate the exported instances object by calling delete instances[name]; add a public cleanup API to encapsulate that behavior: implement and export a function named resetInstance(name) or destroyInstance(name) in the localforage-lru module that safely removes and tears down the named entry from the instances map, then update tests to call resetInstance(name) instead of deleting instances directly; reference the exported instances object and the new resetInstance/destroyInstance symbol when making these changes.src/lib/chain/chain.test.ts (1)
40-42: Add explicit allowlist or generate test fixtures deterministically to avoid future secret scanning issues.The hardcoded private keys at lines 40-42 and 145-146 are test vectors used for deterministic wallet derivation. While gitleaks is not currently in the CI pipeline, these key-like values should be explicitly allowlisted (e.g., with a gitleaks comment or config entry) or generated deterministically in-test. This prevents them from being flagged if secret scanning is added or run locally, and makes their test-fixture nature explicit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chain/chain.test.ts` around lines 40 - 42, The test uses hardcoded secrets (plebbitPrivateKey, authorAddress, walletTimestamp and the other keys around lines 145-146) — either mark them explicitly as test fixtures for secret scanners or generate them deterministically at runtime; update chain.test.ts to replace literal private-key-like strings with a deterministic generator (e.g., derive a seed from a constant test vector and create plebbitPrivateKey/authorAddress via the same derivation used in production) or add a secret-scanner allowlist comment/config adjacent to plebbitPrivateKey and the other test keys documenting they are non-production test vectors so they won’t be flagged by tools like gitleaks.src/hooks/feeds/feeds.ts (1)
199-215: Avoid sorting caller-ownedcommunityAddressesin place.Line 296 and Line 307 call
.sort()on arrays that originate from hook options. That mutates consumer-owned props/state and can reorder the caller’s arrays as a side effect.Suggested fix
function useUniqueSortedArrays(stringsArrays?: string[][]) { return useMemo(() => { const uniqueSorted: string[][] = []; const arrs = stringsArrays ?? []; for (const stringsArray of arrs) { - uniqueSorted.push([...new Set(stringsArray.sort())]); + uniqueSorted.push([...new Set([...stringsArray].sort())]); } return uniqueSorted; }, [stringsArrays]); } function useUniqueSorted(stringsArray?: string[]) { return useMemo(() => { if (!stringsArray) { return []; } - return [...new Set(stringsArray.sort())]; + return [...new Set([...stringsArray].sort())]; }, [stringsArray]); }Also applies to: 291-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/feeds/feeds.ts` around lines 199 - 215, The hook is mutating caller-owned arrays because communityAddresses from feedsOpts are being sorted in place; update the logic that builds communityAddressesArrays (inside useMemo) to copy each feedOptions.communityAddresses before any sorting/processing (e.g., use a shallow clone like Array.from(...) or [...feedOptions.communityAddresses]) so downstream useUniqueSortedArrays and any .sort() calls operate on clones, not the original feedOptions arrays; ensure the change touches the useMemo block that creates communityAddressesArrays and any code paths that call .sort() on those arrays.src/stores/accounts/accounts-actions-internal.test.ts (1)
1097-1203: Add a multi-account regression around role syncing.These cases only cover a single account. If
addCommunityRoleToAccountsCommunities()accidentally mutates every account that has the same community key, this suite still passes. Please add a second account and assert that only the matching author'scommunities[address]entry changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-actions-internal.test.ts` around lines 1097 - 1203, Add a second account to each relevant test to assert changes are scoped to the correct author: in tests exercising addCommunityRoleToAccountsCommunities (referencing accountsActionsInternal.addCommunityRoleToAccountsCommunities and accountsStore), create a second account object with a different author.address and an initial communities map that would be affected if mutations were applied globally; after invoking addCommunityRoleToAccountsCommunities(community), assert that the primary account’s communities[address] changed/removed/added as expected and assert the second account’s communities[address] remains unchanged (or unaffected) to catch regressions where updates are applied to every account sharing the same community key.src/stores/accounts/accounts-actions.test.ts (1)
393-467: Assert account scoping here, not just "no throw".These tests still pass if the community operation mutates the active account instead of
"SubEditAccount","CreateSubAccount", or"DelSubAccount", because they only check that the calls succeed. Since this PR is fixing account-scoping leaks, please assert against the target account'scommunitiesmap before/after the operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-actions.test.ts` around lines 393 - 467, The tests only verify no exceptions but must assert the operation affected the specified named account's communities rather than the active account; for each test that calls publishCommunityEdit/createCommunity/deleteCommunity with an explicit accountName (tests around publishCommunityEdit, createCommunity, deleteCommunity), capture the target accountId from accountsStore.getState().accountNamesToAccountIds["SubEditAccount"/"CreateSubAccount"/"DelSubAccount"], read accountsStore.getState().accounts[targetAccountId].communities (or the appropriate map) before and after the action, and add assertions that the target account's communities were updated (e.g., new community address present or removed) while the original default/other account's communities remain unchanged. Ensure you use the exact symbols publishCommunityEdit, createCommunity, deleteCommunity and accountsStore.getState().accounts/accountNamesToAccountIds when adding these assertions.src/hooks/feeds/feeds.test.ts (1)
1023-1064: This still doesn't assert the active-account feed swap.After
setActiveAccount(), the test only proves the new account's buffered feed fills. The mainuseFeed()result is never checked for the switch-specific regression called out in the inline TODO.I can help reduce this into a focused account-switch regression test if useful.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/feeds/feeds.test.ts` around lines 1023 - 1064, The test currently only verifies the new account's bufferedFeed fills but never asserts that the primary useFeed() result actually swapped to the new account; update the test after calling setActiveAccount(newActiveAccountName) to wait for and assert that rendered.result.current.feed reflects the new active account (e.g., waitFor feed.length > 0 and assert feed[0].cid is a string and either feed[0].cid === bufferedFeed[0].cid or that feed length/content matches the bufferedFeed for the new account), using the existing helpers createAccount, setActiveAccount, useFeed and useBufferedFeeds to locate where to add the assertion. Ensure you replace or remove the TODOed failing block with this explicit assertion so the test fails on a real account-switch regression.src/stores/communities/communities-store.test.ts (1)
97-116: Assert the reject/log side effect here.This test only waits 100ms after wiring a rejected
community.update(). It still passes if the catch/log path disappears, so it is not actually guarding the regression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/communities/communities-store.test.ts` around lines 97 - 116, The test currently just waits 100ms after mocking community.update to reject and never asserts the catch/log side effect; instead spy on the logger used by addCommunityToStore (or fallback to console.error) before calling communitiesStore.getState().addCommunityToStore(address, mockAccount), trigger the mocked rejection via community.update (the existing updateSpy.mockRejectedValueOnce), await the async addCommunityToStore call to complete (do not use a blind setTimeout), and then assert that the logger/console error was called with the expected error or message; restore mocks (updateSpy and createCommunity) afterwards.src/hooks/communities.test.ts (1)
71-80: Restore patched globals infinally.These tests patch global prototypes/store methods and restore them only after the assertions. If a
waitFor()or expectation throws first, the patched behavior leaks into later tests and turns one failure into many.Also applies to: 133-135, 217-250, 253-275, 372-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/communities.test.ts` around lines 71 - 80, The test patches Community.prototype.simulateUpdateEvent (saved as simulateUpdateEvent) and toggles throwOnCommunityUpdateEvent but does not restore the original method if an expectation throws; update the test to restore the original simulateUpdateEvent in a finally block so the prototype is always restored (and reset throwOnCommunityUpdateEvent if needed) after the assertions, referencing the existing simulateUpdateEvent variable and Community.prototype.simulateUpdateEvent to locate and revert the patch; apply the same pattern to the other affected test blocks (lines noted in the comment) to prevent leakage between tests.src/stores/feeds/feeds-store.test.ts (1)
343-396: Make these skip-path regressions observable.These cases only wait for time to pass after mutating store state. If
addCommunitiesPagesOnLowBufferedFeedsCommunitiesPostCountsorupdateFeedsOnFeedsCommunitiesChangeregresses and still fetches/updates, the tests will keep passing. Please assert that page count oraddNextCommunityPageToStorecalls stay unchanged.Also applies to: 448-462
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/feeds/feeds-store.test.ts` around lines 343 - 396, The tests currently just wait after mutating store state but never assert that no fetch/update happened; capture the pre-mutation state (e.g., const beforeCount = rendered.result.current.loadedFeeds[feedName]?.length or spy on the addNextCommunityPageToStore function) then perform the store mutation and wait, and finally assert that the page count is unchanged (expect(loadedFeeds[feedName]?.length).toBe(beforeCount) ) or that the spy was not called; apply the same pattern to both tests (the "skips blocked community" and "skips cache-expired community") and the other similar tests referenced (lines 448-462) to ensure addCommunitiesPagesOnLowBufferedFeedsCommunitiesPostCounts / updateFeedsOnFeedsCommunitiesChange did not trigger addNextCommunityPageToStore or modify page counts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8234227b-c126-4883-9053-801f51a095f1
📒 Files selected for processing (102)
README.mddocs/TODO.mddocs/algorithms.mddocs/clients.mddocs/mock-content.mddocs/schema.mddocs/testing.mdscripts/coverage-gaps.mjssrc/hooks/accounts/accounts.test.tssrc/hooks/accounts/accounts.tssrc/hooks/accounts/index.tssrc/hooks/accounts/utils.test.tssrc/hooks/accounts/utils.tssrc/hooks/actions/actions.test.tssrc/hooks/actions/actions.tssrc/hooks/actions/index.tssrc/hooks/authors/author-avatars.tssrc/hooks/authors/authors.test.tssrc/hooks/authors/authors.tssrc/hooks/authors/index.tssrc/hooks/authors/utils.tssrc/hooks/comments.test.tssrc/hooks/comments.tssrc/hooks/communities.test.tssrc/hooks/communities.tssrc/hooks/feeds/feeds.test.tssrc/hooks/feeds/feeds.tssrc/hooks/feeds/index.tssrc/hooks/replies.test.tssrc/hooks/states.test.tssrc/hooks/states.tssrc/hooks/subplebbits.tssrc/index.tssrc/lib/chain/chain.test.tssrc/lib/chain/index.tssrc/lib/community-address.test.tssrc/lib/community-address.tssrc/lib/debug-utils.tssrc/lib/localforage-lru/localforage-lru.test.tssrc/lib/localforage-lru/localforage-lru.tssrc/lib/plebbit-js/fixtures/markdown-example.tssrc/lib/plebbit-js/plebbit-js-mock-content.donttest.tssrc/lib/plebbit-js/plebbit-js-mock-content.tssrc/lib/plebbit-js/plebbit-js-mock.test.tssrc/lib/plebbit-js/plebbit-js-mock.tssrc/lib/subplebbit-address.test.tssrc/lib/test-utils.tssrc/lib/utils/index.tssrc/lib/utils/utils.test.tssrc/lib/utils/utils.tssrc/lib/validator.tssrc/stores/accounts/account-generator.tssrc/stores/accounts/accounts-actions-internal.test.tssrc/stores/accounts/accounts-actions-internal.tssrc/stores/accounts/accounts-actions.test.tssrc/stores/accounts/accounts-actions.tssrc/stores/accounts/accounts-database.test.tssrc/stores/accounts/accounts-database.tssrc/stores/accounts/accounts-store.test.tssrc/stores/accounts/index.tssrc/stores/accounts/utils.test.tssrc/stores/accounts/utils.tssrc/stores/authors-comments/authors-comments-store.test.tssrc/stores/authors-comments/authors-comments-store.tssrc/stores/authors-comments/index.tssrc/stores/comments/index.tssrc/stores/communities-pages/communities-pages-store.test.tssrc/stores/communities-pages/communities-pages-store.tssrc/stores/communities-pages/index.tssrc/stores/communities/communities-store.test.tssrc/stores/communities/communities-store.tssrc/stores/communities/index.tssrc/stores/feeds/feed-sorter.test.tssrc/stores/feeds/feeds-store.test.tssrc/stores/feeds/feeds-store.tssrc/stores/feeds/index.tssrc/stores/feeds/utils.test.tssrc/stores/feeds/utils.tssrc/stores/replies-pages/index.tssrc/stores/replies-pages/replies-pages-store.test.tssrc/stores/replies-pages/replies-pages-store.tssrc/stores/replies-pages/utils.test.tssrc/stores/replies/index.tssrc/stores/replies/replies-store.test.tssrc/stores/replies/replies-store.tssrc/stores/replies/utils.test.tssrc/stores/replies/utils.tssrc/stores/subplebbits-pages/index.tssrc/stores/subplebbits-pages/subplebbits-pages-store.test.tssrc/stores/subplebbits-pages/subplebbits-pages-store.tssrc/stores/subplebbits/index.tssrc/stores/subplebbits/subplebbits-store.test.tssrc/stores/subplebbits/subplebbits-store.tssrc/types.tstest/browser-e2e/accounts.test.jstest/browser-e2e/communities.test.jstest/browser-e2e/feeds.test.jstest/browser-plebbit-js-mock-content/plebbit-js-mock-content.test.jstest/browser-plebbit-js-mock/accounts.test.jstest/browser-plebbit-js-mock/communities.test.jstest/browser-plebbit-js-mock/feeds.test.jstest/test-server/index.js
💤 Files with no reviewable changes (2)
- src/lib/subplebbit-address.test.ts
- src/hooks/subplebbits.ts
| useEffect(() => { | ||
| if (!communityAddress || !account) { | ||
| return; | ||
| } | ||
| validator.validateUseCommunityArguments(communityAddress, account); | ||
| if (!community && !onlyIfCached) { | ||
| // if community isn't already in store, add it | ||
| addCommunityToStore(communityAddress, account).catch((error: unknown) => | ||
| log.error("useCommunity addCommunityToStore error", { communityAddress, error }), | ||
| ); | ||
| } | ||
| }, [communityAddress, account?.id]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/hooks/communities.ts | head -220Repository: bitsocialnet/bitsocial-react-hooks
Length of output: 9104
Add onlyIfCached to the dependency arrays in both fetch effects.
Both effects read onlyIfCached but omit it from their dependency lists. If a caller toggles onlyIfCached from true (cached-only mode) to false (live mode) for the same community address(es), the effects won't re-run and the hook remains stuck in cached-only mode without fetching fresh data.
Suggested fix
- }, [communityAddress, account?.id]);
+ }, [communityAddress, account?.id, onlyIfCached]);And at line 210:
- }, [addrs.toString(), account?.id]);
+ }, [addrs.toString(), account?.id, onlyIfCached]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!communityAddress || !account) { | |
| return; | |
| } | |
| validator.validateUseCommunityArguments(communityAddress, account); | |
| if (!community && !onlyIfCached) { | |
| // if community isn't already in store, add it | |
| addCommunityToStore(communityAddress, account).catch((error: unknown) => | |
| log.error("useCommunity addCommunityToStore error", { communityAddress, error }), | |
| ); | |
| } | |
| }, [communityAddress, account?.id]); | |
| useEffect(() => { | |
| if (!communityAddress || !account) { | |
| return; | |
| } | |
| validator.validateUseCommunityArguments(communityAddress, account); | |
| if (!community && !onlyIfCached) { | |
| // if community isn't already in store, add it | |
| addCommunityToStore(communityAddress, account).catch((error: unknown) => | |
| log.error("useCommunity addCommunityToStore error", { communityAddress, error }), | |
| ); | |
| } | |
| }, [communityAddress, account?.id, onlyIfCached]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/communities.ts` around lines 42 - 53, The effects that read
onlyIfCached need to include it in their dependency arrays so they re-run when
onlyIfCached changes: update both useEffect hooks that reference
communityAddress, account, validator.validateUseCommunityArguments, and
addCommunityToStore to add onlyIfCached to their dependency lists (alongside
communityAddress and account?.id) so toggling onlyIfCached from true to false
will retrigger the fetch/add logic.
| const { communityAddress, accountName, onlyIfCached } = options ?? {}; | ||
| const account = useAccount({ accountName }); | ||
| const community = useCommunity({ communityAddress, onlyIfCached }); |
There was a problem hiding this comment.
Pass accountName through to useCommunity.
Line 88 loads the backing community from the active account, while Line 87 fetches stats with the requested account. Since owner-community resolution is account-scoped in src/stores/communities/communities-store.ts:34-197, useCommunityStats({ accountName }) can read the wrong community or miss owner-only data.
Suggested fix
- const community = useCommunity({ communityAddress, onlyIfCached });
+ const community = useCommunity({ communityAddress, accountName, onlyIfCached });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/communities.ts` around lines 86 - 88, The community lookup is using
the active account while stats are fetched for a requested account, causing
mismatches; update the call to useCommunity to pass the requested accountName as
well (i.e., include accountName alongside communityAddress and onlyIfCached in
the useCommunity call) so owner-scoped resolution in
useCommunity/useCommunityStats uses the same account context.
| (async () => { | ||
| try { | ||
| setState("resolving"); | ||
| const res = await resolveCommunityAddress(communityAddress, chainProviders); | ||
| setState("succeeded"); | ||
| if (res !== resolvedAddress) { | ||
| setResolvedAddress(res); | ||
| } | ||
| } catch (error: any) { | ||
| setErrors([...errors, error]); | ||
| setState("failed"); | ||
| setResolvedAddress(undefined); | ||
| log.error("useResolvedCommunityAddress resolveCommunityAddress error", { | ||
| communityAddress, | ||
| chainProviders, | ||
| error, | ||
| }); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
Prevent previous resolve attempts from leaking into the current result.
This async branch has no request token/cancel guard, and the success path never clears errors. If communityAddress changes quickly, an older response can overwrite the latest one, and a later success can still return the previous failure in error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/communities.ts` around lines 326 - 344, The IIFE that calls
resolveCommunityAddress can let stale responses overwrite current state and
keeps prior errors; add a request token (e.g., incrementing requestId stored in
a ref or closure) that you capture at call time and check before calling
setState, setResolvedAddress, or setErrors so only the latest request can update
state, and on successful resolution explicitly clear errors (call setErrors([]))
before setting state to "succeeded"; also ensure in the catch you only setErrors
for the matching token and setResolvedAddress(undefined) and setState("failed")
only when the token matches, and include communityAddress/chainProviders in the
log.error as before.
| test("addCidToAccountComment error is logged when it rejects", async () => { | ||
| const addCidSpy = vi.fn().mockRejectedValue(new Error("addCid failed")); | ||
| const accountsGetState = accountsStore.getState; | ||
| (accountsStore as any).getState = () => ({ | ||
| ...accountsGetState(), | ||
| accountsActionsInternal: { addCidToAccountComment: addCidSpy }, | ||
| }); | ||
|
|
||
| const mockCommunity = await mockAccount.plebbit.createCommunity({ | ||
| address: "community address 1", | ||
| }); | ||
| const sortType = "new"; | ||
| const logSpy = vi.spyOn(log, "error").mockImplementation(() => {}); | ||
|
|
||
| act(() => { | ||
| rendered.result.current.addNextCommunityPageToStore(mockCommunity, sortType, mockAccount); | ||
| }); | ||
|
|
||
| await waitFor(() => Object.keys(rendered.result.current.communitiesPages).length > 0); | ||
| await new Promise((r) => setTimeout(r, 100)); | ||
|
|
||
| expect(logSpy).toHaveBeenCalledWith( | ||
| "communitiesPagesStore.addNextCommunityPageToStore addCidToAccountComment error", | ||
| expect.objectContaining({ comment: expect.anything(), error: expect.any(Error) }), | ||
| ); | ||
|
|
||
| logSpy.mockRestore(); | ||
| (accountsStore as any).getState = accountsGetState; | ||
| }); |
There was a problem hiding this comment.
Restore monkey-patched globals in finally.
These tests overwrite accountsStore.getState, utilsMod.default.pageClientsOnStateChange, and MockPages.prototype.getPage, but restoration only happens on the happy path. One failing assertion here will leak the patch into the rest of the suite.
Also applies to: 507-536, 538-573, 625-669, 716-760, 763-796, 798-853
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/communities-pages/communities-pages-store.test.ts` around lines
477 - 505, Wrap each test that temporarily mutates globals (e.g., replacing
accountsStore.getState, utilsMod.default.pageClientsOnStateChange, and
MockPages.prototype.getPage) in a try/finally so the original values are
restored in the finally block; specifically, capture originals into locals (for
example accountsGetState) before overriding, run the test logic in the try, and
in finally always call mockRestore or reassign the captured originals back to
accountsStore.getState, utilsMod.default.pageClientsOnStateChange, and
MockPages.prototype.getPage to prevent leaks across tests (apply the pattern to
tests surrounding addCidToAccountComment, the ones at 507-536, 538-573, 625-669,
716-760, 763-796, 798-853).
| const onCommunityPostsClientsStateChange = | ||
| (communityAddress: string) => | ||
| (clientState: string, clientType: string, sortType: string, clientUrl: string) => { | ||
| communitiesStore.setState((state: CommunitiesState) => { | ||
| // make sure not undefined, sometimes happens in e2e tests | ||
| if (!state.communities[communityAddress]) { | ||
| return {}; | ||
| } | ||
| const client = { state: clientState }; | ||
| const community = { ...state.communities[communityAddress] }; | ||
| community.posts = { ...community.posts }; | ||
| community.posts.clients = { ...community.posts.clients }; | ||
| community.posts.clients[clientType] = { ...community.posts.clients[clientType] }; | ||
| community.posts.clients[clientType][sortType] = { | ||
| ...community.posts.clients[clientType][sortType], | ||
| }; | ||
| community.posts.clients[clientType][sortType][clientUrl] = client; | ||
| return { communities: { ...state.communities, [community.address]: community } }; | ||
| }); |
There was a problem hiding this comment.
Track page-client state on the actual pageType.
fetchPage() only wires pageClientsOnStateChange() when the community client is first created, and the callback always writes into community.posts.clients. If modQueue is fetched after posts, no listener is attached for it; if modQueue is fetched first, its state still lands under posts.
Also applies to: 308-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/communities-pages/communities-pages-store.ts` around lines 265 -
283, The state-update callback onCommunityPostsClientsStateChange always writes
client state into community.posts.clients which ignores the actual pageType
(e.g., "posts" vs "modQueue"), causing listeners wired by
fetchPage/pageClientsOnStateChange to mis-target; update the callback to
accept/derive the pageType and write into community[pageType].clients (or
community.posts if pageType === "posts") instead of hard-coding
community.posts.clients, and ensure the same change is applied to the analogous
handler referenced at lines ~308-318 so listeners for modQueue and other
pageTypes update the correct community.<pageType> structure.
| test("cached community create failure logs to console", async () => { | ||
| const address = "cached-fail-address"; | ||
| const db = localForageLru.createInstance({ name: "plebbitReactHooks-communities" }); | ||
| await db.setItem(address, { address, invalid: "data" }); | ||
|
|
||
| const createCommunityOriginal = mockAccount.plebbit.createCommunity; | ||
| mockAccount.plebbit.createCommunity = vi.fn().mockRejectedValue(new Error("create failed")); | ||
| const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); | ||
|
|
||
| await act(async () => { | ||
| try { | ||
| await communitiesStore.getState().addCommunityToStore(address, mockAccount); | ||
| } catch { | ||
| // expected to throw | ||
| } | ||
| }); | ||
|
|
||
| expect(consoleSpy).toHaveBeenCalledWith( | ||
| "failed plebbit.createCommunity(cachedCommunity)", | ||
| expect.objectContaining({ | ||
| cachedCommunity: expect.any(Object), | ||
| error: expect.any(Error), | ||
| }), | ||
| ); | ||
| consoleSpy.mockRestore(); | ||
| mockAccount.plebbit.createCommunity = createCommunityOriginal; | ||
| }); |
There was a problem hiding this comment.
Restore mocked methods in finally.
Several tests replace mockAccount.plebbit.createCommunity, utils.default.clientsOnStateChange, accountsStore.getState, and communitiesPagesStore.getState, then restore them only after assertions. A failure before cleanup will leak the mock into later tests.
Also applies to: 73-95, 118-149, 152-184, 207-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/communities/communities-store.test.ts` around lines 45 - 71, The
test replaces mocks like mockAccount.plebbit.createCommunity and console error
spy but only restores them after assertions, which can leak into other tests;
update this test (and the others noted) to move restoration into a finally block
so cleanup always runs: capture the original createCommunity in
createCommunityOriginal, set mockAccount.plebbit.createCommunity and create the
consoleSpy as you do now, then wrap the act+assert sequence in try/catch/finally
and restore mockAccount.plebbit.createCommunity = createCommunityOriginal and
call consoleSpy.mockRestore() inside finally; apply the same pattern to other
tests altering utils.default.clientsOnStateChange, accountsStore.getState, and
communitiesPagesStore.getState so all replaced methods are restored even if the
test throws.
|
Pushed commit 2d7612e to address the failing external test-server payload key: the real upstream runtime still expects |
|
Addressed the valid review blockers in 32d117d. What changed:
Local verification after the latest commit:
The remaining local red signal here is tooling-related: yarn test:coverage:hooks-stores still cannot resolve @vitest/coverage-istanbul from this symlinked workspace path, so coverage verification still needs CI or a follow-up tooling fix in this environment. |
1 participant
Addresses CodeRabbit findings and follow-up regressions from the subplebbit→community refactor: broken docs examples (including
role?.role === "owner"fix), account-scoped owner communities, properuseCommunityStatsfailure/uninitialized reporting, case-insensitive.bsonormalization, falsy mock property preservation, safecommunityPostsCacheExpiredguard, per-account page-client caching,UseAccountCommunitiesResultmap type, and realcreateSubplebbitfor the external test server. Includes regression tests.Closes #23
Note
Medium Risk
Mostly CI/test harness and documentation changes, but the new in-memory
localforagedriver and cache/state guards can subtly change unit/e2e behavior and may surface previously hidden persistence-related issues.Overview
Improves CI reliability by building before running e2e suites, ensuring compiled artifacts exist before
test:serverand Playwright runs.Tightens Vitest’s browser-like behavior by setting a JSDOM base URL and replacing
localforagepersistence with a deterministic in-memory driver for tests; adds/adjusts small guards and tests around community cache expiry/state reporting.Updates README and internal docs/scripts to consistently use community terminology (hooks/options/examples, feeds, moderation, schema/algorithms/testing docs) and refreshes coverage tooling match keys accordingly.
Written by Cursor Bugbot for commit 32d117d. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Breaking Changes
New Features
Documentation