Replace directories with living default communities#824
Merged
Conversation
📝 WalkthroughWalkthroughSeedit replaces directory-based subscriptions with versioned starter communities, canonicalizes community and comment routes, adds starter-subscription review actions and localized copy, and removes obsolete directory resolution and synchronization paths. ChangesStarter subscriptions and routing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Account as Account
participant StarterList as Starter community list
participant Notice as Update notice
participant Review as Review page
participant Store as Account persistence
Account->>StarterList: load versioned community list
StarterList-->>Notice: expose revision and delta
Notice->>Review: link to review changes
Review->>Store: add, keep, or replace subscriptions
Store-->>Account: persist updated subscriptions and provenance
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tomcasaburi
marked this pull request as ready for review
July 14, 2026 17:40
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/utils/url-utils.ts (1)
121-143: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winAvoid regex lookbehinds to prevent runtime crashes on older browsers.
Variable-length negative lookbehinds (like
(?<!https?:\/\/[^\s]*)) cause aSyntaxErrorand completely crash the JavaScript parser on older browser versions (e.g., Safari < 16.4 / iOS 15). Since bundlers cannot polyfill regex lookbehinds without fully wrapping regex execution, this will result in a blank screen for those users on load.You can achieve the same boundary-check safely across all browsers by using a capturing group with a negative character class
(^|[^a-zA-Z0-9_/])to assert the boundary and avoid matching inside URLs.🐛 Proposed fix
- // Pattern to match "s/something" or "s/something/comments/something" - // Use negative lookbehind to avoid matching patterns that are already part of URLs - // This prevents matching "s/" that comes after "://" or other URL indicators - const pattern = /(?<!https?:\/\/[^\s]*)\bs\/([a-zA-Z0-9\-.]+(?:\/comments\/[a-zA-Z0-9]{10,100})?)[.,:;!?]*/g; - - return content.replace(pattern, (match, capturedPath) => { + // Pattern to match "s/something" or "s/something/comments/something" + // Use a negative character class `(^|[^a-zA-Z0-9_/])` to enforce a boundary and avoid matching inside URLs. + const pattern = /(^|[^a-zA-Z0-9_/])s\/([a-zA-Z0-9\-.]+(?:\/comments\/[a-zA-Z0-9]{10,100})?)[.,:;!?]*/g; + + return content.replace(pattern, (match, prefix, capturedPath) => { // Remove any trailing punctuation from the captured path const cleanPath = capturedPath.replace(/[.,:;!?]+$/, ''); const fullPattern = `s/${cleanPath}`; if (isValidCommunityPattern(fullPattern)) { // Get the trailing punctuation that was matched but shouldn't be part of the link - const trailingPunctuation = match.slice(fullPattern.length); + const trailingPunctuation = match.slice(prefix.length + fullPattern.length); const postMatch = cleanPath.match(/^([^/]+)\/comments\/([^/]+)$/); const internalPath = postMatch ? getCommunityPostPath(postMatch[1], postMatch[2]) : getCommunityPath(cleanPath); // Convert to markdown link format and preserve trailing punctuation - return `[${fullPattern}](${internalPath})${trailingPunctuation}`; + return `${prefix}[${fullPattern}](${internalPath})${trailingPunctuation}`; } // If not valid, return unchanged return match; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/utils/url-utils.ts` around lines 121 - 143, Replace the variable-length negative lookbehind in the URL-matching pattern used by the surrounding utility with a leading capturing boundary group such as (^|[^a-zA-Z0-9_/]). Update the replacement callback to preserve and reinsert that boundary before the generated markdown link, while keeping community validation, path conversion, and trailing punctuation behavior unchanged.src/views/home/home.tsx (1)
231-283: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDo not sync derived state with effects.
As per coding guidelines, derived state should be computed during render instead of synchronized via
useEffect. By calculatingsubscriptionStateinline, we can remove an unnecessaryuseStatehook and auseEffecthook, which eliminates a double-render cycle. Furthermore, thesafeToShowNoSubscriptionstimeout can be simplified by relying on React's built-in effect cleanup instead of manually tracking a ref.♻️ Proposed refactor for state computation
- const [subscriptionState, setSubscriptionState] = useState<SubscriptionState>('loading'); - const initialLoadTimeoutRef = useRef<NodeJS.Timeout | null>(null); const [safeToShowNoSubscriptions, setSafeToShowNoSubscriptions] = useState(false); useEffect(() => { - return () => { - if (initialLoadTimeoutRef.current) { - clearTimeout(initialLoadTimeoutRef.current); - } - }; - }, []); - - useEffect(() => { if (isCheckingSubscriptions) { setSafeToShowNoSubscriptions(false); return; } - if (!isCheckingSubscriptions && !safeToShowNoSubscriptions) { - initialLoadTimeoutRef.current = setTimeout(() => { - setSafeToShowNoSubscriptions(true); - }, 800); - } - - return () => { - if (initialLoadTimeoutRef.current) { - clearTimeout(initialLoadTimeoutRef.current); - } - }; + const timer = setTimeout(() => { + setSafeToShowNoSubscriptions(true); + }, 800); + + return () => clearTimeout(timer); }, [isCheckingSubscriptions, safeToShowNoSubscriptions, accountAddress]); - useEffect(() => { - if (searchQuery) { - setSubscriptionState('hasSubscriptions'); - return; - } - - if (subscriptions.length > 0 || feed?.length > 0) { - setSubscriptionState('hasSubscriptions'); - return; - } - - if (isCheckingSubscriptions || feed === undefined) { - setSubscriptionState('loading'); - return; - } - - if (!isCheckingSubscriptions && feed?.length === 0 && subscriptions.length === 0 && safeToShowNoSubscriptions) { - setSubscriptionState('noSubscriptions'); - } else { - setSubscriptionState('loading'); - } - }, [isCheckingSubscriptions, subscriptions, feed, safeToShowNoSubscriptions, searchQuery, accountAddress]); + let subscriptionState: SubscriptionState = 'loading'; + if (searchQuery) { + subscriptionState = 'hasSubscriptions'; + } else if (subscriptions.length > 0 || (feed && feed.length > 0)) { + subscriptionState = 'hasSubscriptions'; + } else if (isCheckingSubscriptions || feed === undefined) { + subscriptionState = 'loading'; + } else if (!isCheckingSubscriptions && feed?.length === 0 && subscriptions.length === 0 && safeToShowNoSubscriptions) { + subscriptionState = 'noSubscriptions'; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/home/home.tsx` around lines 231 - 283, Refactor the subscription state logic in the home component to compute subscriptionState directly during render, removing its useState and synchronization useEffect while preserving the existing loading, hasSubscriptions, and noSubscriptions conditions. Simplify the safeToShowNoSubscriptions timeout effect by removing initialLoadTimeoutRef and relying on the effect cleanup to clear the scheduled timeout, including cleanup when dependencies change or the component unmounts.Source: Coding guidelines
src/components/subscribe-button/subscribe-button.tsx (1)
25-44: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCommit the provenance update only after the unsubscribe path succeeds.
useSubscribe().unsubscribe()swallows its own errors, so thisawaitdoes not guarantee the community was actually removed. BecausesetAccount()persistsseeditStarterSubscriptionsfirst, a failed/stale unsubscribe can leave provenance out of sync with the real subscription list. Move the provenance write behind the unsubscribe path or make the failure visible before callingonUnsubscribe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/subscribe-button/subscribe-button.tsx` around lines 25 - 44, The handleSubscribe function currently persists updated provenance before confirming unsubscribe succeeded. Update the subscribed === true branch to perform unsubscribe first, then compute and persist the leaveStarterSubscription provenance only after that path succeeds, and invoke onUnsubscribe only after the successful update; preserve the existing address and account guards.
🧹 Nitpick comments (7)
src/components/markdown/markdown.tsx (1)
95-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the internal link condition.
The regex match
href.match(/^\/s\/[^/]+(\/comments\/[^/]+)?$/)is redundant because any string that satisfies this pattern will already evaluate totruefor the precedinghref.startsWith('/s/')check. You can safely remove it to simplify the condition.♻️ Proposed refactor
- if (href.startsWith('`#/`') || href.startsWith('/#/') || href.startsWith('/s/') || href.match(/^\/s\/[^/]+(\/comments\/[^/]+)?$/)) { + if (href.startsWith('`#/`') || href.startsWith('/#/') || href.startsWith('/s/')) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/markdown/markdown.tsx` at line 95, In the internal-link condition, remove the redundant href.match(...) regex alternative and retain the existing href.startsWith checks, including href.startsWith('/s/'). Keep the surrounding link-handling behavior unchanged.src/components/post/comment-tools/comment-tools.tsx (1)
204-223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
cidchecks inside the truthy branches.Since
permalinkButton,contextButton, andfullCommentsButtonalready use an outer ternary (cid ? ... : ...) that ensurescidis truthy before rendering the<Link>, the innercid ? ... : ...checks in thetoprops and the!cidfallback logic inonClickare dead code.Consider simplifying the code to remove the redundancy.
🧹 Proposed cleanup
- const permalinkButton = cid ? ( - <Link to={cid ? getCommunityPostPath(communityAddress, cid) : `/profile/${index}`} onClick={(e) => !cid && e.preventDefault()}> - permalink - </Link> - ) : ( - <span>permalink</span> - ); - - const contextButton = cid ? ( - <Link to={cid ? (hasContext ? `${getCommunityPostPath(communityAddress, cid)}/?context=3` : getCommunityPostPath(communityAddress, cid)) : `/profile/${index}`}> - {t('context')} - </Link> - ) : ( - <span>{t('context')}</span> - ); - - const fullCommentsButton = cid ? ( - <Link to={cid ? getCommunityPostPath(communityAddress, postCid!) : `/profile/${index}`}> - {t('full_comments')} {comment?.replyCount ? `(${comment?.replyCount})` : ''} - </Link> - ) : ( + const permalinkButton = cid ? ( + <Link to={getCommunityPostPath(communityAddress, cid)}> + permalink + </Link> + ) : ( + <span>permalink</span> + ); + + const contextButton = cid ? ( + <Link to={hasContext ? `${getCommunityPostPath(communityAddress, cid)}/?context=3` : getCommunityPostPath(communityAddress, cid)}> + {t('context')} + </Link> + ) : ( + <span>{t('context')}</span> + ); + + const fullCommentsButton = cid ? ( + <Link to={getCommunityPostPath(communityAddress, postCid!)}> + {t('full_comments')} {comment?.replyCount ? `(${comment?.replyCount})` : ''} + </Link> + ) : (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/post/comment-tools/comment-tools.tsx` around lines 204 - 223, Remove the redundant inner cid checks from permalinkButton, contextButton, and fullCommentsButton. Within each truthy branch, use the already-validated cid directly for the Link to values and remove the !cid fallback logic from permalinkButton’s onClick, while preserving the existing false branches and context query behavior.src/hooks/use-auto-subscribe.test.ts (1)
4-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOnly the “brand-new account” branch of
computeStarterAccountis covered.The
currentProvenance-merge branch, themigration.changedbranch, and theisKnownExistingAccount(no-bootstrap) branch incomputeStarterAccountare untested here, even though they are the core logic exercised bypersistStarterAccount's divergence-rebase path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-auto-subscribe.test.ts` around lines 4 - 22, The tests for computeStarterAccount only cover brand-new account bootstrapping. Add focused cases covering the currentProvenance merge path, the migration.changed path, and the isKnownExistingAccount no-bootstrap path, including assertions for subscriptions and seeditStarterSubscriptions; ensure these scenarios reflect the divergence-rebase behavior used by persistStarterAccount.src/components/topbar/topbar.tsx (1)
208-213: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize derived collections to avoid recreating Sets on every render.
Creating a
Setand filtering the array during every render cycle can cause unnecessary overhead, especially if the component re-renders frequently. Consider wrapping this derived state in auseMemohook.⚡ Proposed refactor
- const subscriptionSet = new Set(subscriptions ?? []); - const filteredCommunityAddresses: string[] = []; - for (const { address } of defaultCommunities) { - if (!subscriptionSet.has(address)) filteredCommunityAddresses.push(address); - } + const filteredCommunityAddresses = useMemo(() => { + const subscriptionSet = new Set(subscriptions ?? []); + const addresses: string[] = []; + for (const { address } of defaultCommunities) { + if (!subscriptionSet.has(address)) addresses.push(address); + } + return addresses; + }, [subscriptions, defaultCommunities]); const activeCommunityAddress = resolveCommunityRouteAddress(params.communityAddress);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/topbar/topbar.tsx` around lines 208 - 213, Memoize the derived subscription Set and filtered community addresses in the topbar component using useMemo, with subscriptions and defaultCommunities as dependencies. Preserve the existing filtering behavior and keep activeCommunityAddress resolution outside the memoized collection logic.src/views/starter-subscriptions/starter-subscriptions.tsx (1)
22-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse declarative
<title>instead ofuseEffect.In React 19, you can render document head tags directly inside your components, eliminating the need for
useEffectto managedocument.title. As per coding guidelines, we should reconsider any newuseEffectusage and avoid it when a better alternative exists.♻️ Proposed refactor
- useEffect(() => { - document.title = `${t('starter_communities')} - seedit`; - }, [t]); - const toggleAddress = (address: string) => { setSelectedAddresses((current) => { const next = new Set(current); if (next.has(address)) next.delete(address); else next.add(address); return next; }); }; return ( <main className={styles.page}> + <title>{t('starter_communities')} - seedit</title> <div className={styles.infobar}> <h1>{t('starter_communities')}</h1>You can then also safely remove
useEffectfrom thereactimports at the top of the file:import { useState } from 'react';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/starter-subscriptions/starter-subscriptions.tsx` around lines 22 - 38, Replace the document.title useEffect in the starter subscriptions component with a declarative React 19 <title> element using the existing t('starter_communities') translation, and remove the now-unused useEffect import while preserving the page title format.Source: Coding guidelines
src/views/communities/communities.tsx (2)
376-407: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the
defaultsByAddressMap pattern here too.
AllDefaultSubplebbitsandAllAccountSubplebbitsbuild adefaultsByAddressMap for O(1) lookups (Lines 430, 482), butSubscriberSubplebbitsstill doesdefaultCommunities.find(...)per item in both the filter and map callbacks (Lines 386, 395). Worth aligning for consistency and to avoid the repeated O(n) scan.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/communities/communities.tsx` around lines 376 - 407, Create a defaultsByAddress Map from defaultCommunities before building communityElements, then reuse it for the default community lookup in both the filter and map callbacks. Replace each defaultCommunities.find call while preserving the existing tag filtering and CommunityItem props.
430-446: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate tag-filtering Map construction.
The
defaultsByAddress/taggedAddressesconstruction block is nearly identical betweenAllDefaultSubplebbits(Lines 430-436) andAllAccountSubplebbits(Lines 482-488), differing only by the source array variable name. Consider extracting a small helper (e.g.buildTaggedAddressIndex(defaultCommunities, currentTag)) undersrc/hooks/or a shared util to avoid the copy-paste.As per coding guidelines, "avoid copy-paste logic across components; extract shared logic into custom hooks under
src/hooks/."Also applies to: 482-506
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/communities/communities.tsx` around lines 430 - 446, Extract the duplicated defaults-by-address and tagged-address construction from AllDefaultSubplebbits and AllAccountSubplebbits into a shared helper or custom hook, such as buildTaggedAddressIndex, under the existing hooks or shared-util area. Update both components to call it with their respective default community arrays and currentTag, while preserving the existing NSFW/tag matching behavior and returned address index.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/use-auto-subscribe.ts`:
- Around line 42-62: Move the initial accountsDatabase.addAccount write in
persistStarterAccount until after confirming whether the store account still
matches sourceAccount. When the account has diverged, compute the rebased
account first and persist that account only if starterStateChanged indicates a
change; preserve the existing store update behavior and avoid persisting stale
nextAccount.
In `@src/hooks/use-default-subscriptions.ts`:
- Around line 162-166: Memoize all derived return values in
useDefaultSubscriptionAddresses and the adjacent metadata and tag-returning
hooks using useMemo. At src/hooks/use-default-subscriptions.ts lines 162-166,
memoize the mapped address array with subscriptions as its dependency; at lines
167-170, memoize the metadata object using the individual list fields as
dependencies; and at line 172, memoize the deduplicated, sorted tag array with
subscriptions as its dependency.
In `@src/views/post-page/post-page.tsx`:
- Around line 279-298: Normalize postCommunityAddress using the same
community-address resolution applied to routeCommunityAddress before the
redirect comparison in the post-page effect. Use the normalized value for both
the mismatch check and generated getCommunityPostPath target, while preserving
the existing pending-post behavior and dependency updates.
---
Outside diff comments:
In `@src/components/subscribe-button/subscribe-button.tsx`:
- Around line 25-44: The handleSubscribe function currently persists updated
provenance before confirming unsubscribe succeeded. Update the subscribed ===
true branch to perform unsubscribe first, then compute and persist the
leaveStarterSubscription provenance only after that path succeeds, and invoke
onUnsubscribe only after the successful update; preserve the existing address
and account guards.
In `@src/lib/utils/url-utils.ts`:
- Around line 121-143: Replace the variable-length negative lookbehind in the
URL-matching pattern used by the surrounding utility with a leading capturing
boundary group such as (^|[^a-zA-Z0-9_/]). Update the replacement callback to
preserve and reinsert that boundary before the generated markdown link, while
keeping community validation, path conversion, and trailing punctuation behavior
unchanged.
In `@src/views/home/home.tsx`:
- Around line 231-283: Refactor the subscription state logic in the home
component to compute subscriptionState directly during render, removing its
useState and synchronization useEffect while preserving the existing loading,
hasSubscriptions, and noSubscriptions conditions. Simplify the
safeToShowNoSubscriptions timeout effect by removing initialLoadTimeoutRef and
relying on the effect cleanup to clear the scheduled timeout, including cleanup
when dependencies change or the component unmounts.
---
Nitpick comments:
In `@src/components/markdown/markdown.tsx`:
- Line 95: In the internal-link condition, remove the redundant href.match(...)
regex alternative and retain the existing href.startsWith checks, including
href.startsWith('/s/'). Keep the surrounding link-handling behavior unchanged.
In `@src/components/post/comment-tools/comment-tools.tsx`:
- Around line 204-223: Remove the redundant inner cid checks from
permalinkButton, contextButton, and fullCommentsButton. Within each truthy
branch, use the already-validated cid directly for the Link to values and remove
the !cid fallback logic from permalinkButton’s onClick, while preserving the
existing false branches and context query behavior.
In `@src/components/topbar/topbar.tsx`:
- Around line 208-213: Memoize the derived subscription Set and filtered
community addresses in the topbar component using useMemo, with subscriptions
and defaultCommunities as dependencies. Preserve the existing filtering behavior
and keep activeCommunityAddress resolution outside the memoized collection
logic.
In `@src/hooks/use-auto-subscribe.test.ts`:
- Around line 4-22: The tests for computeStarterAccount only cover brand-new
account bootstrapping. Add focused cases covering the currentProvenance merge
path, the migration.changed path, and the isKnownExistingAccount no-bootstrap
path, including assertions for subscriptions and seeditStarterSubscriptions;
ensure these scenarios reflect the divergence-rebase behavior used by
persistStarterAccount.
In `@src/views/communities/communities.tsx`:
- Around line 376-407: Create a defaultsByAddress Map from defaultCommunities
before building communityElements, then reuse it for the default community
lookup in both the filter and map callbacks. Replace each
defaultCommunities.find call while preserving the existing tag filtering and
CommunityItem props.
- Around line 430-446: Extract the duplicated defaults-by-address and
tagged-address construction from AllDefaultSubplebbits and AllAccountSubplebbits
into a shared helper or custom hook, such as buildTaggedAddressIndex, under the
existing hooks or shared-util area. Update both components to call it with their
respective default community arrays and currentTag, while preserving the
existing NSFW/tag matching behavior and returned address index.
In `@src/views/starter-subscriptions/starter-subscriptions.tsx`:
- Around line 22-38: Replace the document.title useEffect in the starter
subscriptions component with a declarative React 19 <title> element using the
existing t('starter_communities') translation, and remove the now-unused
useEffect import while preserving the page title format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a70c705b-d458-40c7-88b8-1608390127c6
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (110)
PRODUCT.mdREADME.mddocs/agent-runs/starter-subscriptions/feature-list.jsondocs/agent-runs/starter-subscriptions/progress.mdpackage.jsonpublic/llms-full.txtpublic/llms.txtpublic/translations/ar/default.jsonpublic/translations/bn/default.jsonpublic/translations/cs/default.jsonpublic/translations/da/default.jsonpublic/translations/de/default.jsonpublic/translations/el/default.jsonpublic/translations/en/default.jsonpublic/translations/es/default.jsonpublic/translations/fa/default.jsonpublic/translations/fi/default.jsonpublic/translations/fil/default.jsonpublic/translations/fr/default.jsonpublic/translations/he/default.jsonpublic/translations/hi/default.jsonpublic/translations/hu/default.jsonpublic/translations/id/default.jsonpublic/translations/it/default.jsonpublic/translations/ja/default.jsonpublic/translations/ko/default.jsonpublic/translations/mr/default.jsonpublic/translations/nl/default.jsonpublic/translations/no/default.jsonpublic/translations/pl/default.jsonpublic/translations/pt/default.jsonpublic/translations/ro/default.jsonpublic/translations/ru/default.jsonpublic/translations/sq/default.jsonpublic/translations/sv/default.jsonpublic/translations/te/default.jsonpublic/translations/th/default.jsonpublic/translations/tr/default.jsonpublic/translations/uk/default.jsonpublic/translations/ur/default.jsonpublic/translations/vi/default.jsonpublic/translations/zh/default.jsonscripts/sync-directories.jssrc/app.tsxsrc/components/author-sidebar/author-sidebar.tsxsrc/components/header/header.tsxsrc/components/markdown/markdown.tsxsrc/components/notification-handler/notification-handler.tsxsrc/components/post/comment-tools/comment-tools.tsxsrc/components/post/post.tsxsrc/components/post/thumbnail/thumbnail.tsxsrc/components/reply/reply.tsxsrc/components/search-bar/search-bar.tsxsrc/components/sidebar/sidebar.module.csssrc/components/sidebar/sidebar.tsxsrc/components/starter-subscriptions-notice/starter-subscriptions-notice.module.csssrc/components/starter-subscriptions-notice/starter-subscriptions-notice.tsxsrc/components/subscribe-button/subscribe-button.tsxsrc/components/topbar/topbar.tsxsrc/data/seedit-directories/seedit-askseedit-directory.jsonsrc/data/seedit-directories/seedit-aww-directory.jsonsrc/data/seedit-directories/seedit-funny-directory.jsonsrc/data/seedit-directories/seedit-gaming-directory.jsonsrc/data/seedit-directories/seedit-interestingasfuck-directory.jsonsrc/data/seedit-directories/seedit-memes-directory.jsonsrc/data/seedit-directories/seedit-news-directory.jsonsrc/data/seedit-directories/seedit-pics-directory.jsonsrc/data/seedit-directories/seedit-todayilearned-directory.jsonsrc/data/seedit-directories/seedit-videos-directory.jsonsrc/data/seedit-starter-communities.jsonsrc/data/vendored-directory-lists.tssrc/hooks/use-auto-subscribe.test.tssrc/hooks/use-auto-subscribe.tssrc/hooks/use-default-subscriptions.test.tssrc/hooks/use-default-subscriptions.tssrc/hooks/use-directory-list.tssrc/hooks/use-directory-winners.tssrc/hooks/use-expanded-subscriptions.tssrc/hooks/use-is-nsfw-community.tssrc/hooks/use-now-seconds.tssrc/hooks/use-resolved-community-address.tssrc/hooks/use-starter-subscriptions.tssrc/hooks/use-time-filter.tssrc/lib/utils/community-freshness-utils.tssrc/lib/utils/community-route-utils.test.tssrc/lib/utils/community-route-utils.tssrc/lib/utils/directory-codes.test.tssrc/lib/utils/directory-codes.tssrc/lib/utils/directory-list-utils.test.tssrc/lib/utils/directory-list-utils.tssrc/lib/utils/legacy-default-subscriptions.test.tssrc/lib/utils/legacy-default-subscriptions.tssrc/lib/utils/starter-account.tssrc/lib/utils/starter-subscriptions.test.tssrc/lib/utils/starter-subscriptions.tssrc/lib/utils/url-utils.test.tssrc/lib/utils/url-utils.tssrc/lib/utils/view-utils.tssrc/views/about/about.tsxsrc/views/author/author.tsxsrc/views/communities/communities.module.csssrc/views/communities/communities.tsxsrc/views/community-settings/community-data-editor/community-data-editor.tsxsrc/views/community-settings/community-settings.tsxsrc/views/community/community.tsxsrc/views/home/home.tsxsrc/views/post-page/post-page.tsxsrc/views/starter-subscriptions/starter-subscriptions.module.csssrc/views/starter-subscriptions/starter-subscriptions.tsxsrc/views/submit-page/submit-page.tsx
💤 Files with no reviewable changes (24)
- src/data/seedit-directories/seedit-memes-directory.json
- src/data/seedit-directories/seedit-pics-directory.json
- src/data/seedit-directories/seedit-todayilearned-directory.json
- src/data/seedit-directories/seedit-funny-directory.json
- src/data/seedit-directories/seedit-news-directory.json
- src/data/seedit-directories/seedit-gaming-directory.json
- scripts/sync-directories.js
- src/data/seedit-directories/seedit-askseedit-directory.json
- src/data/seedit-directories/seedit-videos-directory.json
- src/lib/utils/directory-codes.test.ts
- src/lib/utils/directory-codes.ts
- src/data/seedit-directories/seedit-interestingasfuck-directory.json
- src/hooks/use-expanded-subscriptions.ts
- src/data/seedit-directories/seedit-aww-directory.json
- src/hooks/use-directory-list.ts
- src/lib/utils/directory-list-utils.test.ts
- src/views/communities/communities.module.css
- src/hooks/use-resolved-community-address.ts
- src/hooks/use-now-seconds.ts
- src/lib/utils/community-freshness-utils.ts
- src/hooks/use-directory-winners.ts
- src/lib/utils/directory-list-utils.ts
- src/data/vendored-directory-lists.ts
- src/components/sidebar/sidebar.module.css
Member
Author
|
Addressed the valid review findings in four focused commits:
Deliberately declined the low-value memo/Map cleanup suggestions where React Compiler already handles caching, plus broad pre-existing barrel-import/large-component cleanup. Legacy /c/ route aliases remain intentionally removed: this PR establishes /comments/ as the only CID route before Seedit has users or shared legacy permalinks. Local verification on Node 22.12.0: 72 tests, type-check, lint, production build, Knip, AI workflow parity, generated llms files, forge-config import, React Doctor changed-scope review, and Chrome/Firefox/WebKit desktop+mobile browser checks. |
Member
Author
|
Follow-up review fixes are pushed as separate commits:
Verification on Node 22.12.0: full test suite (75 tests), type-check, lint, production build, knip, workflow alignment, generated LLM catalogs, diff check, and Electron forge-config import all pass. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7528dc0. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/views/home/home.tsx (1)
26-28: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnstable empty-array reference feeds
useMemodeps.
account?.subscriptions ?? []creates a new array on every render whileaccount/account.subscriptionsis nullish, which invalidates thefeedOptions(Lines 76-92) andfooterProps(Lines 197-229) memos by reference during the loading window even though the content is unchanged.use-starter-subscriptions.tsalready hoists a module-levelEMPTY_SUBSCRIPTIONSconstant for this same reason — consider mirroring that here.♻️ Stabilize the empty fallback
+const EMPTY_SUBSCRIPTIONS: string[] = []; + const Home = () => { const { t } = useTranslation(); const account = useAccount(); - const subscriptions = account?.subscriptions ?? []; + const subscriptions = account?.subscriptions ?? EMPTY_SUBSCRIPTIONS;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/home/home.tsx` around lines 26 - 28, Stabilize the empty subscriptions fallback in the home view by defining or reusing a module-level empty subscriptions constant, then update the subscriptions assignment to use it instead of an inline [] fallback. Keep communityAddresses and the existing feedOptions and footerProps memo dependencies unchanged.src/lib/utils/starter-account-persistence.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the public account API over
dist/stores/*.@bitsocial/bitsocial-react-hooksexposes non-hook account helpers such assetAccount,setActiveAccount,importAccount, andexportAccount, so this utility should avoid depending on internaldist/stores/accounts/*paths unless it truly needs store internals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/utils/starter-account-persistence.ts` around lines 1 - 2, Replace the internal accountsDatabase and accountsStore imports in starter-account-persistence with the public account helpers exposed by `@bitsocial/bitsocial-react-hooks`, using setAccount, setActiveAccount, importAccount, or exportAccount as appropriate. Preserve the utility’s existing behavior and retain direct store imports only if a required operation cannot be performed through the public API.src/hooks/use-auto-subscribe.ts (1)
19-19: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse selectors to avoid subscribing to the entire store.
Destructuring values directly from the
useAutoSubscribeStore()hook causes this component to subscribe to all state changes within the store. This means any time an account is added or removed from the checking list, every component usinguseAutoSubscribewill unnecessarily re-render. As per coding guidelines, reviewing changes with React best practices implies avoiding unnecessary re-renders. Use individual selectors to isolate subscriptions.⚡ Proposed refactor
- const { addCheckingAccount, removeCheckingAccount, isCheckingAccount } = useAutoSubscribeStore(); + const addCheckingAccount = useAutoSubscribeStore((state) => state.addCheckingAccount); + const removeCheckingAccount = useAutoSubscribeStore((state) => state.removeCheckingAccount); + const isCheckingAccount = useAutoSubscribeStore((state) => state.isCheckingAccount);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-auto-subscribe.ts` at line 19, Update useAutoSubscribe to select addCheckingAccount, removeCheckingAccount, and isCheckingAccount individually from useAutoSubscribeStore instead of destructuring the entire store, so components subscribe only to the state slices they use.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/use-auto-subscribe.ts`:
- Around line 38-42: Move the hasExistingSeeditAccountState call for
isKnownExistingAccount inside the existing try block in processAutoSubscribe,
keeping migratingAccounts.add before the try as appropriate. Ensure
localStorage.getItem errors are caught by the existing error handling so
removeCheckingAccount still executes and the UI cannot remain stuck.
In `@src/hooks/use-starter-subscriptions.ts`:
- Around line 33-48: Handle persistence failures at both call sites: in
src/hooks/use-starter-subscriptions.ts lines 33-48, add catch handling inside
persist and expose the failure through appropriate error state or user feedback
for addSelected, keepCurrent, and replacePrevious; in
src/components/subscribe-button/subscribe-button.tsx lines 26-49, wrap the await
persistStarterAccountUpdate call in try/catch, prevent an unhandled rejection,
and only invoke onUnsubscribe after a successful write.
---
Nitpick comments:
In `@src/hooks/use-auto-subscribe.ts`:
- Line 19: Update useAutoSubscribe to select addCheckingAccount,
removeCheckingAccount, and isCheckingAccount individually from
useAutoSubscribeStore instead of destructuring the entire store, so components
subscribe only to the state slices they use.
In `@src/lib/utils/starter-account-persistence.ts`:
- Around line 1-2: Replace the internal accountsDatabase and accountsStore
imports in starter-account-persistence with the public account helpers exposed
by `@bitsocial/bitsocial-react-hooks`, using setAccount, setActiveAccount,
importAccount, or exportAccount as appropriate. Preserve the utility’s existing
behavior and retain direct store imports only if a required operation cannot be
performed through the public API.
In `@src/views/home/home.tsx`:
- Around line 26-28: Stabilize the empty subscriptions fallback in the home view
by defining or reusing a module-level empty subscriptions constant, then update
the subscriptions assignment to use it instead of an inline [] fallback. Keep
communityAddresses and the existing feedOptions and footerProps memo
dependencies unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48e3c59c-8567-4d90-a371-cddde570d7e2
📒 Files selected for processing (22)
electron/before-pack.jssrc/components/sidebar/sidebar.tsxsrc/components/starter-subscriptions-notice/starter-subscriptions-notice.module.csssrc/components/starter-subscriptions-notice/starter-subscriptions-notice.tsxsrc/components/subscribe-button/subscribe-button.tsxsrc/hooks/use-auto-subscribe.test.tssrc/hooks/use-auto-subscribe.tssrc/hooks/use-default-subscriptions.test.tssrc/hooks/use-default-subscriptions.tssrc/hooks/use-starter-subscriptions.tssrc/lib/utils/community-route-utils.test.tssrc/lib/utils/community-route-utils.tssrc/lib/utils/starter-account-persistence.tssrc/lib/utils/starter-account.tssrc/lib/utils/starter-subscriptions.test.tssrc/lib/utils/starter-subscriptions.tssrc/lib/utils/url-utils.test.tssrc/lib/utils/url-utils.tssrc/views/about/about.tsxsrc/views/home/home.tsxsrc/views/post-page/post-page.tsxsrc/views/starter-subscriptions/starter-subscriptions.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- src/lib/utils/community-route-utils.test.ts
- src/lib/utils/community-route-utils.ts
- src/views/starter-subscriptions/starter-subscriptions.tsx
- src/views/post-page/post-page.tsx
- src/components/starter-subscriptions-notice/starter-subscriptions-notice.module.css
- src/lib/utils/starter-subscriptions.ts
- src/lib/utils/url-utils.ts
- src/views/about/about.tsx
- src/hooks/use-default-subscriptions.ts
- src/components/sidebar/sidebar.tsx
- src/lib/utils/starter-subscriptions.test.ts
Comment on lines
+33
to
+48
| const persist = (computeResult: (currentAccount: StarterAccount) => StarterSubscriptionsResult) => { | ||
| if (!account) return Promise.resolve(); | ||
| setSaving(true); | ||
| return Promise.resolve() | ||
| .then(() => | ||
| persistStarterAccountUpdate(account.id, (currentAccount) => { | ||
| const result = computeResult(currentAccount); | ||
| return { | ||
| ...currentAccount, | ||
| subscriptions: result.subscriptions, | ||
| seeditStarterSubscriptions: result.provenance, | ||
| }; | ||
| }), | ||
| ) | ||
| .finally(() => setSaving(false)); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No error handling around persistStarterAccountUpdate calls. Both sites call persistStarterAccountUpdate (an IndexedDB write via accountsDatabase.addAccount) without a .catch/try-catch, and downstream UI call sites (starter-subscriptions.tsx) invoke these with void, so a failed write produces only an unhandled promise rejection — no error state, no user feedback, and the "saving" UI just quietly resets as if the action succeeded.
src/hooks/use-starter-subscriptions.ts#L33-L48: wrap thepersistStarterAccountUpdatecall in a.catchinsidepersist(e.g. capture into anerror/saveErrorstate) soaddSelected/keepCurrent/replacePreviousfailures are surfaced instead of silently discarded.src/components/subscribe-button/subscribe-button.tsx#L26-L49: wrap theawait persistStarterAccountUpdate(...)call in try/catch so an unsubscribe failure doesn't leave an unhandled rejection and can optionally avoid firingonUnsubscribeon failure.
📍 Affects 2 files
src/hooks/use-starter-subscriptions.ts#L33-L48(this comment)src/components/subscribe-button/subscribe-button.tsx#L26-L49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/use-starter-subscriptions.ts` around lines 33 - 48, Handle
persistence failures at both call sites: in
src/hooks/use-starter-subscriptions.ts lines 33-48, add catch handling inside
persist and expose the failure through appropriate error state or user feedback
for addSelected, keepCurrent, and replacePrevious; in
src/components/subscribe-button/subscribe-button.tsx lines 26-49, wrap the await
persistStarterAccountUpdate call in try/catch, prevent an unhandled rejection,
and only invoke onUnsubscribe after a successful write.
Member
Author
|
The four refreshed review findings are addressed in separate commits:
Latest Node 22.12.0 verification: 10 test files / 77 tests, type-check, lint, production build, Knip, AI workflow parity, generated catalogs, diff check, Electron forge-config import, and changed-scope React Doctor. React Doctor reports only the previously declined barrel-import and pre-existing large-Post warnings. |
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
1 participant
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
.bsoshorthand plus a versioned, remotely refreshed default-community list/comments/<cid>for community and author URLsSafety and update behavior
The external canonical payload is published in
bitsocialnet/listsmaster.Commit structure
/comments/<cid>route migrationVerification
yarn type-checkyarn lintyarn test --run— 60 testsyarn buildgit diff --checkPostcomponent warning remainsNote
High Risk
Touches core subscription state, routing, and a breaking permalink URL change across the app, with migration logic that must not drop manual subscriptions or accept stale/malformed remote list updates.
Overview
Replaces Seedit directory codes with direct community-address subscriptions, a versioned default-community list, and user-controlled update handling.
Subscriptions and navigation now go through
community-route-utils: single-label.bsonames can use shorthand in/s/routes, and permalinks move from/c/<cid>to/comments/<cid>(no legacy redirects). Directory vendored data, sync tooling, sidebar directory copy, and directory-based subscribe behavior are removed; existing directory-code subscriptions migrate to fixed addresses viause-auto-subscribe/ starter provenance on the account.Default communities load from a remote
seedit-default-subscriptions.jsonpayload (revision-gated, vendored fallback). When the list changes, a home notice links to/communities/defaultsso users can add selected communities, keep current subs, or switch to the latest defaults—only Seedit-managed defaults can be dropped on replace/unsubscribe.Docs and i18n follow the new model (
PRODUCT.md, README, LLM indexes, 35 locales);sync:directoriesis dropped frompackage.json.Reviewed by Cursor Bugbot for commit 88ab9cb. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
/comments/<cid>across the app.