Use LocationData type in MapComponent props type.#42
Use LocationData type in MapComponent props type.#42izadoesdev merged 4 commits intodatabuddy-analytics:mainfrom
Conversation
|
@sbansal1999 is attempting to deploy a commit to the Databuddy Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis change centralizes and standardizes type definitions for location-based analytics data by introducing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
43dd8e9 to
190d59a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/dashboard/components/analytics/map-component.tsx (1)
89-96:perCapitavalue is off by six orders of magnitude
perCapitaValueis calculated asvisitors / population, yet the tooltip label says “per million people”.
Multiply by1_000_000(or divide population by 1e6) so the number matches the label.-const perCapitaValue = population > 0 ? item.count / population : 0; +const perCapitaValue = + population > 0 ? (item.count * 1_000_000) / population : 0;
🧹 Nitpick comments (2)
apps/dashboard/app/(main)/websites/[id]/map/page.tsx (1)
29-32: Consider extractingLocationDatato a shared types module instead of exporting from a page fileImporting types directly from a page-level module (
map/page.tsx) forces lower-level components such asMapComponentto depend on UI route files, creating an inverted dependency and increasing the risk of circular imports during refactors.
MoveCountryData,RegionData, andLocationDatainto a colocatedtypes.ts(or@/types/analytics.ts) and re-export from there; components and pages can then import from that stable location.apps/dashboard/components/analytics/map-component.tsx (1)
50-57: Defaulting divisor to1hides “all-zero” data cases
validCountries.reduce(... ) || 1prevents division-by-zero but silently turns 0 visitors into 100 × 0/1 = 0%, which is fine, yet the magic constant obscures intent. Consider an explicit guard:-const totalVisitors = validCountries.reduce((sum, c) => sum + c.visitors, 0) || 1; +const totalVisitorsRaw = validCountries.reduce((sum, c) => sum + c.visitors, 0); +const totalVisitors = totalVisitorsRaw === 0 ? 1 : totalVisitorsRaw;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/app/(main)/websites/[id]/map/page.tsx(1 hunks)apps/dashboard/components/analytics/map-component.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
190d59a to
cb4a98c
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
move the types into packages/shared please so everything is uniform |
e6f1de6 to
95775bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/app/(main)/websites/[id]/map/page.tsx(1 hunks)apps/dashboard/components/analytics/map-component.tsx(7 hunks)apps/dashboard/tsconfig.json(1 hunks)packages/shared/src/types/website.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(main)/websites/[id]/map/page.tsx
- apps/dashboard/components/analytics/map-component.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/01-MUST-DO.mdc)
Always ensure type-safety, don't use type: any unless needed, when creating APIs, responses, or components, create proper interfaces and make them in the shared types folders where it fits best, not in the same file
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use TypeScript namespaces.
Don't use the TypeScript directive @ts-ignore.
Don't use any type.
Don't use implicit any type on variable declarations.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't declare empty interfaces.
Use export type for types.
Use import type for types.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Files:
packages/shared/src/types/website.ts
**/*.{js,jsx,ts,tsx,css,scss}
📄 CodeRabbit Inference Engine (.cursor/rules/01-MUST-DO.mdc)
Always use rounded, not rounded-xl or rounded-md, always rounded
Files:
packages/shared/src/types/website.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/01-MUST-DO.mdc)
**/*.{js,jsx,ts,tsx}: use console properly, like console.error, console.time, console.json, console.table, etc
Use Dayjs NEVER date-fns, and Tanstack query for hooks, NEVER SWR
Use ONLY Zod V4 from zod/v4 never zod 3 from zod
use json.stringify() when adding debugging
**/*.{js,jsx,ts,tsx}: Usebun <file>instead ofnode <file>orts-node <file>
Do not use dotenv; Bun automatically loads .env files
Do not useexpress; useBun.serve()for HTTP servers
Do not usebetter-sqlite3; usebun:sqlitefor SQLite
Do not useioredis; useBun.redisfor Redis
Do not usepgorpostgres.js; useBun.sqlfor Postgres
Do not usews; use built-inWebSocket
PreferBun.fileovernode:fs's readFile/writeFile
UseBun.$instead of execa for running shell commands
Files:
packages/shared/src/types/website.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,jsx,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't use global eval().
Don't use var.
Don't use console.
Don't use debugger.
Don't assign directly to document.cookie.
Don't use duplicate case labels.
Don't use duplicate class members.
Don't use duplicate function parameter names.
Don't use empty block statements and static blocks.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Use for...of statements instead of Array.forEach.
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of for loops when you don't need initializer and update expressions.
Don't reassign const variables.
Don't use constant expressions in conditions.
Don't use Math.min and Math.max to clamp values when the result is constant.
Don't return a value from a constructor.
Don't use empty character classes in regular expression literals.
Don't use empty destructuring patterns.
Don't call global object properties as functions.
Don't declare functions and vars that are accessible outside their block.
Don't use variables and function parameters before they're declared.
Don't use 8 and 9 escape sequences in string literals.
Don't use literal numbers that lose precision.
Don't use duplicate conditions in if-else-if chains.
Don't use two keys with the same name inside objects...
Files:
packages/shared/src/types/website.ts
**/*.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts}: Make sure to use the "use strict" directive in script files.
Don't have redundant "use strict".
Make sure to use the "use strict" directive in script files.
Files:
packages/shared/src/types/website.ts
**/*.{html,ts,css}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuild
Files:
packages/shared/src/types/website.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: databuddy-analytics/Databuddy#0
File: .cursor/rules/01-MUST-DO.mdc:0-0
Timestamp: 2025-07-29T15:40:18.511Z
Learning: Applies to **/shared/types/**/*.{ts,tsx} : When creating interfaces for APIs, responses, or components, make them in the shared types folders where it fits best, not in the same file
Learnt from: CR
PR: databuddy-analytics/Databuddy#0
File: .cursor/rules/01-MUST-DO.mdc:0-0
Timestamp: 2025-07-29T15:40:18.511Z
Learning: Applies to **/*.{ts,tsx} : Always ensure type-safety, don't use type: any unless needed, when creating APIs, responses, or components, create proper interfaces and make them in the shared types folders where it fits best, not in the same file
Learnt from: CR
PR: databuddy-analytics/Databuddy#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-29T15:42:01.054Z
Learning: Applies to **/*.{ts,tsx} : Use import type for types.
Learnt from: CR
PR: databuddy-analytics/Databuddy#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-29T15:42:01.054Z
Learning: Applies to **/*.{ts,tsx} : Use export type for types.
Learnt from: CR
PR: databuddy-analytics/Databuddy#0
File: .cursor/rules/01-MUST-DO.mdc:0-0
Timestamp: 2025-07-29T15:40:18.511Z
Learning: Handle complex data transformations independently of React. Keep these modules decoupled from React for improved modularity and testability.
packages/shared/src/types/website.ts (1)
Learnt from: CR
PR: databuddy-analytics/Databuddy#0
File: .cursor/rules/01-MUST-DO.mdc:0-0
Timestamp: 2025-07-29T15:40:18.511Z
Learning: Applies to /shared/types//*.{ts,tsx} : When creating interfaces for APIs, responses, or components, make them in the shared types folders where it fits best, not in the same file
🧬 Code Graph Analysis (1)
packages/shared/src/types/website.ts (1)
apps/dashboard/app/(main)/websites/[id]/_components/tabs/audience-tab.tsx (1)
GeographicEntry(31-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
| export interface CountryData { | ||
| country: string; | ||
| country_code?: string; | ||
| visitors: number; | ||
| pageviews: number; | ||
| } | ||
|
|
||
| export interface RegionData { | ||
| country: string; | ||
| visitors: number; | ||
| pageviews: number; | ||
| } | ||
|
|
||
| export interface LocationData { | ||
| countries: CountryData[]; | ||
| regions: RegionData[]; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify consistency with existing geographic data types.
The new location data types are well-structured and properly placed in the shared types folder. However, there appears to be a similar GeographicEntry interface in apps/dashboard/app/(main)/websites/[id]/_components/tabs/audience-tab.tsx that has overlapping but different structure.
Consider consolidating these types to avoid duplication and ensure consistency across the codebase:
🏁 Script executed:
#!/bin/bash
# Search for other geographic or location-related interfaces that might need consolidation
rg -A 10 -B 2 "interface.*Geographic|interface.*Location|interface.*Country|interface.*Region" --type tsLength of output: 4422
# End of script🏁 Script executed:
#!/bin/bash
# Find barrel index in shared types
echo "Barrel files in packages/shared/src/types:"
fd -t f index.ts packages/shared/src/types
# Show content of barrel if exists
if [ -f packages/shared/src/types/index.ts ]; then
echo "Content of packages/shared/src/types/index.ts:"
sed -n '1,200p' packages/shared/src/types/index.ts
else
echo "No barrel index.ts found in packages/shared/src/types"
fi
# Locate and display the GeographicEntry interface in audience-tab.tsx
AUD_TAB_PATH=$(fd -t f audience-tab.tsx apps)
echo "Audience tab file path: $AUD_TAB_PATH"
if [ -n "$AUD_TAB_PATH" ]; then
echo "GeographicEntry snippet:"
rg -A 5 -B 5 "interface GeographicEntry" $AUD_TAB_PATH
else
echo "audience-tab.tsx not found"
fiLength of output: 1518
Consolidate and de-duplicate geographic data types
The new CountryData, RegionData and LocationData in
packages/shared/src/types/website.ts are well-placed, but we now have three overlapping declarations:
• In packages/shared/src/types/metrics.ts:
– export interface CountryData extends BaseMetricData {}
– export interface RegionData extends BaseMetricData {}
• In packages/shared/src/types/website.ts:
– export interface CountryData { country, country_code?, visitors, pageviews }
– export interface RegionData { country, visitors, pageviews }
• In
apps/dashboard/app/(main)/websites/[id]/_components/tabs/audience-tab.tsx:
– interface GeographicEntry { name, visitors, pageviews, percentage, country_code? }
To avoid shadowing, confusion and duplication:
- Rename metric-specific types to
MetricCountryData/MetricRegionData. - Move
GeographicEntryinto your shared folder (e.g. extendCountryDataorRegionDataand addpercentage). - Remove the local
GeographicEntryinaudience-tab.tsxand import the shared type.
Suggested diffs:
# packages/shared/src/types/metrics.ts
-export interface CountryData extends BaseMetricData {}
-export interface RegionData extends BaseMetricData {}
+export interface MetricCountryData extends BaseMetricData {}
+export interface MetricRegionData extends BaseMetricData {}# apps/dashboard/.../audience-tab.tsx
- interface GeographicEntry {
- name: string;
- visitors: number;
- pageviews: number;
- percentage: number;
- country_code?: string;
- country_name?: string;
- }
+ import type { CountryData } from 'packages/shared/src/types/website';
+
+ interface GeographicEntry extends CountryData {
+ percentage: number;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/shared/src/types/website.ts around lines 50 to 66, there are
overlapping and duplicated geographic data types across multiple files causing
confusion. Rename the metric-specific interfaces in
packages/shared/src/types/metrics.ts to MetricCountryData and MetricRegionData
to clarify their purpose. Move the GeographicEntry interface from
apps/dashboard/app/(main)/websites/[id]/_components/tabs/audience-tab.tsx into
the shared types folder, extending either CountryData or RegionData and adding
the percentage field. Then remove the local GeographicEntry definition in
audience-tab.tsx and import the shared type instead to consolidate and
de-duplicate these types.
There was a problem hiding this comment.
@izadoesdev do you think we can remove the type CountryData and RegionData defined in packages/shared/src/types/metrics.ts as they are not being used anywhere?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@sbansal1999 if it's not used anywhere, remove it, just double check everything works in map and builds
There was a problem hiding this comment.
instead of removing them; what if we rename the types one in metrics (I feel they will come handy; as they are related to metrics.)
something like this
diff --git a/packages/shared/src/types/metrics.ts b/packages/shared/src/types/metrics.ts
index 2a11c1e9..3dab55d8 100644
--- a/packages/shared/src/types/metrics.ts
+++ b/packages/shared/src/types/metrics.ts
@@ -7,17 +7,17 @@ export interface BaseMetricData {
pageviews: number;
}
-export interface DeviceTypeData extends BaseMetricData {}
+export interface DeviceTypeMetricData extends BaseMetricData {}
-export interface BrowserData extends BaseMetricData {}
+export interface BrowserMetricData extends BaseMetricData {}
-export interface CountryData extends BaseMetricData {}
+export interface CountryMetricData extends BaseMetricData {}
-export interface RegionData extends BaseMetricData {}
+export interface RegionMetricData extends BaseMetricData {}
-export interface PageData extends BaseMetricData {}
+export interface PageMetricData extends BaseMetricData {}
-export interface ReferrerData extends BaseMetricData {}
+export interface ReferrerMetricData extends BaseMetricData {}There was a problem hiding this comment.
yup, this is done; I have pushed the changes
This helps in using them later from other places.
95775bf to
b3ea452
Compare
The current logic will give minimum value as Infinity incase the array has all values as 0.
b3ea452 to
c1aa1ec
Compare
dd8d5d9 to
c38e813
Compare
This PR replaces
anywith theLocationData type that is defined where theMapComponentcomponent is used. Also, removed some otherany` definitions as types are inferred now.Summary by CodeRabbit
New Features
Refactor