refactor: consolidate mock data into centralized lib/mock/ with factory pattern - #247
Conversation
|
Someone is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
@Shadow-MMN Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
More reviews will be available in 38 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR consolidates mock data files from the top level into a structured ChangesMock Data Layer Reorganization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
lib/mock/discover.ts (1)
507-508: ⚡ Quick winBuild
oldMockBountiesfromoldMockProjectsto avoid dataset drift.Using
makeOldProjects()twice creates two independent project arrays. ReuseoldMockProjectsso both exports stay in sync by construction.💡 Suggested fix
export const oldMockProjects = makeOldProjects(); -export const oldMockBounties = makeOldBounties(makeOldProjects()); +export const oldMockBounties = makeOldBounties(oldMockProjects);🤖 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 `@lib/mock/discover.ts` around lines 507 - 508, The export currently calls makeOldProjects() twice which creates two different arrays; change the oldMockBounties export to reuse the already-created oldMockProjects so they stay in sync by calling makeOldBounties(oldMockProjects) instead of makeOldProjects() – update the export of oldMockBounties to reference the existing oldMockProjects variable (symbols: oldMockProjects, oldMockBounties, makeOldProjects, makeOldBounties).
🤖 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 `@app/api/leaderboard/route.ts`:
- Around line 7-8: Validate and sanitize the parsed pagination inputs before
using them: after obtaining page and limit via parseInt(searchParams.get("page")
|| "1") and parseInt(searchParams.get("limit") || "10"), ensure they are
positive integers (fallback to 1 for page and 10 for limit if NaN, <=0, or
non-integer), optionally clamp limit to a reasonable max (e.g., 100) to prevent
abuse, and then use these sanitized variables in the ranking/pagination math
(references: page, limit, parseInt, searchParams).
In `@app/api/leaderboard/top/route.ts`:
- Line 6: The parsed "count" value (const count =
parseInt(searchParams.get("count") || "5")) can be NaN or non-positive; update
the route handler to sanitize it by validating that count is a finite positive
integer and falling back to a safe default (e.g., 5) if not, and optionally
clamp to a reasonable max to preserve top-N semantics; apply the same validation
wherever "count" is parsed (the other occurrence around the second parseInt at
line 12) so the endpoint always returns a valid top-N parameter.
In `@lib/mock/index.ts`:
- Around line 1-9: The file re-exports mockProjects and mockBounties twice
causing a potential TS2308 conflict: remove or rename the duplicate re-exports
in lib/mock/index.ts so the names from "./projects" and "./bounties" are not
shadowed by the aliases from "./discover"; specifically, either delete the block
exporting oldMockProjects as mockProjects and oldMockBounties as mockBounties,
or change that export to export { oldMockProjects, oldMockBounties } (keeping
their original names) so the unique symbols mockProjects and mockBounties come
only from "./projects" and "./bounties".
In `@lib/mock/leaderboard.ts`:
- Around line 61-75: In getMockLeaderboard validate and normalize pagination
inputs before computing start: ensure page and limit are positive integers
(e.g., coerce to integers and use Math.max(1, ...) for page and Math.max(1, ...)
for limit) so start = (page-1)*limit and slice arguments are always non-negative
and safe; update the logic that computes start, paginated and any downstream
uses to use these normalized values.
- Around line 35-56: The function makeMockLeaderboardData can call Array.from
with a negative length when count < 10; guard count before generating extras by
computing extra = Math.max(0, count - 10) or branching so you only call
Array.from when count > 10. Update the spread that builds extra contributors
(the Array.from call that creates makeMockContributor entries) to use extra
instead of count - 10 (or early-return/slice the hardcoded list when count <=
10) and ensure indices/IDs still start at 11 (use i + 11) so makeMockContributor
and ReputationTier usage remains correct.
In `@lib/services/withdrawal.ts`:
- Around line 25-29: Add a lower-bound validation before creating withdrawals:
besides checking amount > mockWalletWithAssets.balance, validate that amount is
a positive number and that amount > fee (or explicitly that netAmount = amount -
fee > 0); if these fail set result.valid = false, push descriptive errors into
result.errors (e.g., "Invalid amount" or "Amount must exceed fee") and add new
blocker flags (e.g., result.blockers.invalidAmount or
result.blockers.feeTooHigh). Update both the initial balance/amount check (where
result.valid and result.blockers.insufficientBalance are set) and the later
netAmount computation (around netAmount usage at the netAmount check) to prevent
creating withdrawals with netAmount <= 0, referencing amount, fee, netAmount,
result, and mockWalletWithAssets.balance.
- Around line 114-116: The getHistory method returns the internal array by
reference (MOCK_WITHDRAWALS[userId]) which allows callers to mutate service
state; change WithdrawalService.getHistory to return a shallow copy of the array
(e.g., create a new array from MOCK_WITHDRAWALS[userId] or return an empty array
copy when absent) so callers get a safe copy instead of the original, ensuring
modifications to the returned value do not alter MOCK_WITHDRAWALS.
---
Nitpick comments:
In `@lib/mock/discover.ts`:
- Around line 507-508: The export currently calls makeOldProjects() twice which
creates two different arrays; change the oldMockBounties export to reuse the
already-created oldMockProjects so they stay in sync by calling
makeOldBounties(oldMockProjects) instead of makeOldProjects() – update the
export of oldMockBounties to reference the existing oldMockProjects variable
(symbols: oldMockProjects, oldMockBounties, makeOldProjects, makeOldBounties).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f1773c6-8ffb-4566-bb26-efa2984c9efb
📒 Files selected for processing (24)
app/api/leaderboard/route.tsapp/api/leaderboard/top/route.tsapp/api/leaderboard/user/[userId]/route.tsapp/discover/page.tsxapp/projects/[id]/page.tsxapp/projects/page.tsxapp/wallet/page.tsxcomponents/bounty-detail/bounty-detail-client.tsxcomponents/search-command.tsxlib/mock-bounty.tslib/mock-data.tslib/mock-leaderboard.tslib/mock-model4.tslib/mock-project.tslib/mock-wallet.tslib/mock/bounties.tslib/mock/discover.tslib/mock/index.tslib/mock/leaderboard.tslib/mock/model4.tslib/mock/projects.tslib/mock/wallet.tslib/services/withdrawal.tslib/store.ts
💤 Files with no reviewable changes (6)
- lib/mock-data.ts
- lib/mock-wallet.ts
- lib/mock-model4.ts
- lib/mock-leaderboard.ts
- lib/mock-project.ts
- lib/mock-bounty.ts
| export * from "./bounties"; | ||
| export * from "./leaderboard"; | ||
| export * from "./wallet"; | ||
| export * from "./projects"; | ||
| export * from "./model4"; | ||
| export { | ||
| oldMockProjects as mockProjects, | ||
| oldMockBounties as mockBounties, | ||
| } from "./discover"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting potentially conflicting exports..."
sed -n '1,120p' lib/mock/index.ts
echo
echo "Checking if upstream modules export the same symbols:"
rg -n --type=ts 'export\s+(const|function|type|interface)\s+mock(Project|Bounties|Projects|Bounty|Bounties)|export\s*\{[^}]*mockProjects|export\s*\{[^}]*mockBounties' lib/mockRepository: boundlessfi/bounties
Length of output: 550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- lib/mock/discover.ts (lines around oldMockProjects/oldMockBounties) ----"
sed -n '1,220p' lib/mock/discover.ts | sed -n '/oldMockProjects/,/oldMockBounties/p'
echo
echo "---- Full matching lines for oldMockProjects/oldMockBounties ----"
rg -n --type=ts "oldMockProjects|oldMockBounties" lib/mock/discover.ts
echo
echo "---- Ensure discover exports them (export declarations) ----"
rg -n --type=ts "export\s+(const|function|type|interface)\s+oldMock(Project|Bounties|Projects|Bounty|Bounties)" lib/mock/discover.ts || trueRepository: boundlessfi/bounties
Length of output: 521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for TypeScript duplicate export diagnostics or known handling..."
rg -n --hidden --no-ignore-vcs --type=ts --type=tsx --type=js --type=json --type=md \
"(TS2308|already exported a member named|Duplicate|conflicting exports)" . || true
echo
echo "Searching for any existing mention of mockProjects/mockBounties duplicate export errors..."
rg -n --hidden --no-ignore-vcs --type=ts --type=tsx --type=js --type=json \
"mockProjects.*already exported|mockBounties.*already exported|mockProjects.*duplicate|mockBounties.*duplicate" . || trueRepository: boundlessfi/bounties
Length of output: 305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- lib/mock/index.ts (full) ----"
cat -n lib/mock/index.ts
echo
echo "---- Check for scripts/typecheck in package.json ----"
if [ -f package.json ]; then
cat -n package.json | sed -n '1,200p'
else
echo "package.json not found"
fi
echo
echo "---- Search for duplicate export-related TS diagnostics ----"
rg -n --hidden --no-ignore-vcs -g'*.ts' -g'*.tsx' -g'*.d.ts' -g'*.md' \
"(TS2308|already exported a member named|conflict.*exported|Duplicate|re-export.*conflict)" . || true
echo
echo "---- Search for consumers importing mockProjects/mockBounties from lib/mock ----"
rg -n --hidden --no-ignore-vcs -g'*.ts' -g'*.tsx' \
"from ['\"](.*/)?lib/mock['\"]|from ['\"]@?/.*/mock['\"]|from ['\"](.*/)?mock['\"]" . || trueRepository: boundlessfi/bounties
Length of output: 41151
🌐 Web query:
TypeScript TS2308 already exported a member named re-export conflicts with exported declaration when using export * and export { ... as ... } from different modules
💡 Result:
TS2308 (“Module {0} has already exported a member named '{1}'. Consider explicitly re-exporting to resolve the ambiguity”) happens when, in a single module (file), you create two competing exports with the same name—commonly by combining export * from '...' with another re-export that also exports that same identifier. What’s going on - export * from 'other-module' re-exports all exported members from the target module. - If, in the same re-exporting file, you also re-export that same identifier again (either directly, or via export {... as... } from...), TypeScript sees the name collision in the current module’s export surface and raises TS2308, telling you to disambiguate by explicitly re-exporting [1]. Concrete example of the collision - If module “apples” and module “oranges” both export Fruit, and you write: - export * from 'apples' - export * from 'oranges' then TS2308 is thrown because Fruit is now being exported twice from the same index/re-export module [1]. How to fix - Explicitly choose the source of the conflicting name: - export * from 'apples' - export * from 'oranges' - export { Fruit } from 'oranges' (or from 'apples') This “explicit re-export” strategy is exactly what the cited guidance recommends to resolve the ambiguity [1]. - Or, avoid export * for the conflicting surface by explicitly listing the exports you want to re-export (so you can prevent duplicates) [1]. - If you need to bring both versions in, rename one using export { OriginalName as NewName } from... so the exported identifiers no longer collide (the mechanism for using “as” renames is standard re-export guidance) [2]. So, to your query’s specific wording - The root issue is duplicate exported identifier names within the re-exporting module; the trigger pattern is “export *” plus another re-export that produces the same exported name [1]. Even if the second re-export uses export {... as... }, it will still fail if the resulting exported name matches something already exported via export * (i.e., it still produces a duplicate name).
Citations:
- 1: https://stackoverflow.com/questions/41293108/how-to-do-re-export-with-overrides
- 2: https://medium.com/@robinviktorsson/typescript-and-es-modules-best-practices-for-imports-and-exports-9ce200e75a88
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- tsconfig.json (and variants) ----"
ls -la tsconfig*.json 2>/dev/null || true
for f in tsconfig*.json; do
[ -f "$f" ] && echo "===== $f =====" && sed -n '1,200p' "$f"
done
echo
echo "---- next.config.* ----"
ls -la next.config.* 2>/dev/null || true
for f in next.config.*; do
[ -f "$f" ] && echo "===== $f =====" && sed -n '1,200p' "$f"
doneRepository: boundlessfi/bounties
Length of output: 1105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Running TypeScript typecheck (tsc)..."
# Use local npx/tsc from node_modules (no installs in sandbox)
npx --yes tsc -p tsconfig.json --noEmit --pretty false 2>&1 | tee /tmp/tsc.out | head -n 200
echo
echo "Filtered: TS2308 or mockProjects/mockBounties diagnostics"
rg -n "TS2308|already exported a member named|mockProjects|mockBounties" /tmp/tsc.out || trueRepository: boundlessfi/bounties
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Running TypeScript typecheck (tsc) with redirected output..."
npx --yes tsc -p tsconfig.json --noEmit --pretty false > /tmp/tsc.out 2>&1 || true
echo "---- tsc output (first 120 lines) ----"
head -n 120 /tmp/tsc.out
echo
echo "---- Filter for duplicate export / TS2308 ----"
rg -n "TS2308|already exported a member named|mockProjects|mockBounties" /tmp/tsc.out || trueRepository: boundlessfi/bounties
Length of output: 18481
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "lib/mock/index\.ts|lib/mock/" /tmp/tsc.out || trueRepository: boundlessfi/bounties
Length of output: 46
Avoid duplicate mockProjects/mockBounties re-exports in lib/mock/index.ts.
lib/mock/index.ts already re-exports mockProjects/mockBounties via export * from "./projects" and export * from "./bounties", and then re-exports the same names again via export { oldMockProjects as mockProjects, oldMockBounties as mockBounties } from "./discover". This export-surface duplication is the classic pattern that can trigger TypeScript’s TS2308 (“already exported a member named …”); in the current repo typecheck output it didn’t surface, but the cleanup is still recommended to prevent ambiguity.
🤖 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 `@lib/mock/index.ts` around lines 1 - 9, The file re-exports mockProjects and
mockBounties twice causing a potential TS2308 conflict: remove or rename the
duplicate re-exports in lib/mock/index.ts so the names from "./projects" and
"./bounties" are not shadowed by the aliases from "./discover"; specifically,
either delete the block exporting oldMockProjects as mockProjects and
oldMockBounties as mockBounties, or change that export to export {
oldMockProjects, oldMockBounties } (keeping their original names) so the unique
symbols mockProjects and mockBounties come only from "./projects" and
"./bounties".
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
Clean refactor. Every domain has its own file under lib/mock/, factory pattern is consistently applied with makeX(overrides?) signatures, and the barrel index re-exports everything so consumers can do either from "@/lib/mock" or from "@/lib/mock/projects". All six old lib/mock-*.ts files are deleted, no stragglers.
Typecheck and lint clean. Merging this in. Thanks.
2 participants
Files changed:
Closes #210
Summary by CodeRabbit