feat: add NexusDateRangePicker component for date range selection in …#122
feat: add NexusDateRangePicker component for date range selection in …#122wabinyai wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe frontend adds historical report generation with date and site selection, analytics data retrieval and normalization, shared readings prefetching, and weekly/monthly comparison support across report summaries and charts. ChangesHistorical reporting flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReportsPage
participant ReportDataModal
participant loadHistoricalReportData
participant AirQoProxy
participant AnalyticsDataDownload
participant buildSiteReportData
ReportsPage->>ReportDataModal: submit date range and site selection
ReportDataModal->>loadHistoricalReportData: request analytics data
loadHistoricalReportData->>AirQoProxy: POST analytics/data-download
AirQoProxy->>AnalyticsDataDownload: forward request with server token
AnalyticsDataDownload-->>AirQoProxy: return report records
AirQoProxy-->>loadHistoricalReportData: return response payload
loadHistoricalReportData->>buildSiteReportData: build normalized site data
buildSiteReportData-->>ReportsPage: update historical report state
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/reports/page.tsx (1)
1723-1728: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"Weekly Change" summary card isn't period-aware.
The value already switches on
comparisonPeriodviacalculateAveragePercentageChange, but this card's title stays hardcoded, so monthly reports show a monthly percentage under a "Weekly Change" label (the Results section at Line 1554 was updated, this one was missed).✏️ Proposed fix
<SummaryCard - title="Weekly Change" + title={comparisonPeriod === "monthly" ? "Monthly Change" : "Weekly Change"} value={`${calculateAveragePercentageChange(filteredData).toFixed(2)}%`}🤖 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 `@frontend/src/app/reports/page.tsx` around lines 1723 - 1728, The SummaryCard for the “Weekly Change” display uses a hardcoded title despite calculateAveragePercentageChange respecting comparisonPeriod. Update its title in the SummaryCard block to derive the period label from comparisonPeriod, matching the period-aware Results section while leaving value, icon, and trend unchanged.
🤖 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 `@frontend/src/app/reports/page.tsx`:
- Around line 98-101: Update the comparisonPeriod calculation in the report page
to require the selected date span to satisfy the intended duration or
month-over-month precondition before returning "monthly"; do not rely solely on
startDate and endDate crossing calendar months. Preserve "weekly" for short
cross-month ranges such as Jan 30–Feb 2, while retaining monthly mode for valid
full month-over-month selections.
- Around line 451-459: Update the filter effect around filterSites and
setFilteredData so it no longer depends on selectedSite, while retaining the
stale-selection cleanup logic. Remove selectedSite from the dependency array,
ensuring single-site flows that set filteredData are not overwritten when the
selection changes.
In `@frontend/src/components/charts/AirQualityChart.tsx`:
- Around line 567-569: Update the comparison Y-axis domain calculation in
AirQualityChart so empty chartData produces a valid finite domain instead of
spreading an empty allValues array into Math.max. Preserve the existing 10%
headroom for non-empty data and use an appropriate zero-based fallback when no
comparison values exist.
---
Outside diff comments:
In `@frontend/src/app/reports/page.tsx`:
- Around line 1723-1728: The SummaryCard for the “Weekly Change” display uses a
hardcoded title despite calculateAveragePercentageChange respecting
comparisonPeriod. Update its title in the SummaryCard block to derive the period
label from comparisonPeriod, matching the period-aware Results section while
leaving value, icon, and trend 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3599197a-d118-46e5-ba1c-698808cd0383
📒 Files selected for processing (9)
frontend/src/app/api/airqo/[...path]/route.tsfrontend/src/app/layout.tsxfrontend/src/app/reports/page.tsxfrontend/src/components/AppDataWarmup.tsxfrontend/src/components/charts/AirQualityChart.tsxfrontend/src/components/reports/NexusDateRangePicker.tsxfrontend/src/components/reports/ReportDataModal.tsxfrontend/src/lib/types.tsfrontend/src/services/apiService.tsx
| useEffect(() => { | ||
| const result = filterSites(siteData, filters) | ||
| const result = filterSites(customReportData ?? siteData, filters) | ||
|
|
||
| setFilteredData(result) | ||
| // Reset selected site if it's no longer in filtered data | ||
| if (selectedSite && !result.some((site) => getSiteSelectionId(site) === getSiteSelectionId(selectedSite))) { | ||
| setSelectedSite(null) | ||
| } | ||
| }, [filters, siteData, selectedSite]) | ||
| }, [customReportData, filters, siteData, selectedSite]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching reports/page.tsx:"
fd -a 'page\.tsx$' . | sed 's#^\./##' | grep 'frontend/src/app/reports/page.tsx' || true
echo
echo "Line count:"
wc -l frontend/src/app/reports/page.tsx
echo
echo "Relevant effect/hooks context:"
sed -n '1,80p' frontend/src/app/reports/page.tsx
echo
sed -n '420,470p' frontend/src/app/reports/page.tsx
echo
echo "FilteredData/selectedSite usages:"
rg -n "setFilteredData|setSelectedSite|selectedSite|filteredData|SiteCard" frontend/src/app/reports/page.tsxRepository: airqo-platform/code-samples
Length of output: 12535
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate SiteCard maybe same file:"
rg -n "onSelect|function SiteCard|const SiteCard|setFilteredData|setSelectedSite" frontend/src/app/reports/page.tsx
echo
echo "Read likely SiteCard section:"
sed -n '1820,1910p' frontend/src/app/reports/page.tsx
echo
echo "Read filterSites helper if present:"
sed -n '1,140p' frontend/src/app/reports/page.tsxRepository: airqo-platform/code-samples
Length of output: 10443
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ReportContent opening and state declarations:"
sed -n '80,520p' frontend/src/app/reports/page.tsx
echo
echo "Single-device report generation callers around filters reset/useReportOnPage:"
sed -n '1140,1265p' frontend/src/app/reports/page.tsx
echo
echo "SiteCard signature/body relevant section:"
sed -n '2068,2275p' frontend/src/app/reports/page.tsx
echo
echo "JS semantic probe for state-update + deps after render, then deps array includes changed state:"
node - <<'JS'
// Model the specific flow:
// - render uses filteredData = [site], selectedSite = null
// - onSelect sets filteredData = [site], selectedSite = site
// - changeDetector includes selectedSite, selectedSite is in previous deps, so effect runs
// - effect body sets filteredData = filterSites(customReportData ?? siteData, filters)
let siteData = [{name: "network site"}]
let customReportData = null
let filters = {}
let filteredData = [
{name: "drilled site"},
]
let selectedSite = {name: "drilled site"}
function filterSites(sites) {
return sites;
}
function prevSelectedSite() { return undefined } // selectedSite was not previously selected
const changedDeps = []
const currentDeps = [customReportData, filters, siteData, selectedSite]
const depsDifffer = filteredData.length;
let overwriteTriggered = false
if (false) {
// omitted
}
selectedSite && selectedSite !== prevSelectedSite() && selectedSite !== undefined && (overwriteTriggered = true)
if (overwriteTriggered) {
filteredData = filterSites(customReportData ?? siteData, filters)
}
console.log(JSON.stringify({filteredData, selectedSite, overwriteTriggered}))
JSRepository: airqo-platform/code-samples
Length of output: 31132
Remove the single-site reset from the filter effect.
setSelectedSite is only called inside single-site flows that also set filteredData to that one site. When that selection state changes, this effect re-runs and replaces it with filterSites(customReportData ?? siteData, filters), so the drilldown view renders the full filtered network instead of the selected site. Keep the stale-selection cleanup there, but exclude selectedSite from the dependency array so the filter effect doesn’t overwrite those flows.
🐛 Proposed fix
useEffect(() => {
const result = filterSites(customReportData ?? siteData, filters)
setFilteredData(result)
// Reset selected site if it's no longer in filtered data
if (selectedSite && !result.some((site) => getSiteSelectionId(site) === getSiteSelectionId(selectedSite))) {
setSelectedSite(null)
}
- }, [customReportData, filters, siteData, selectedSite])
+ }, [customReportData, filters, siteData])📝 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(() => { | |
| const result = filterSites(siteData, filters) | |
| const result = filterSites(customReportData ?? siteData, filters) | |
| setFilteredData(result) | |
| // Reset selected site if it's no longer in filtered data | |
| if (selectedSite && !result.some((site) => getSiteSelectionId(site) === getSiteSelectionId(selectedSite))) { | |
| setSelectedSite(null) | |
| } | |
| }, [filters, siteData, selectedSite]) | |
| }, [customReportData, filters, siteData, selectedSite]) | |
| useEffect(() => { | |
| const result = filterSites(customReportData ?? siteData, filters) | |
| setFilteredData(result) | |
| // Reset selected site if it's no longer in filtered data | |
| if (selectedSite && !result.some((site) => getSiteSelectionId(site) === getSiteSelectionId(selectedSite))) { | |
| setSelectedSite(null) | |
| } | |
| }, [customReportData, filters, siteData]) |
🤖 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 `@frontend/src/app/reports/page.tsx` around lines 451 - 459, Update the filter
effect around filterSites and setFilteredData so it no longer depends on
selectedSite, while retaining the stale-selection cleanup logic. Remove
selectedSite from the dependency array, ensuring single-site flows that set
filteredData are not overwritten when the selection changes.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/components/charts/AirQualityChart.tsx (1)
754-767: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the Y-axis domain against empty
chartData(still open).With no sites carrying the selected period's averages,
chartDatais empty, soMath.max(...[])returns-InfinityandcomparisonYAxisDomainbecomes[0, -Infinity]— an invalid Recharts domain. This was flagged in a prior review and remains unaddressed; the monthly-mode addition in this PR widens the surface for hitting it (e.g., sites withoutmonthlyAveragespopulated).🛡️ Proposed fix
const allValues = chartData.flatMap((item) => [item.current ?? 0, item.previous ?? 0]) - const maxComparisonValue = Math.max(...allValues) + const maxComparisonValue = allValues.length > 0 ? Math.max(...allValues) : 0 const comparisonYAxisDomain = [0, Math.ceil(maxComparisonValue * 1.1)]🤖 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 `@frontend/src/components/charts/AirQualityChart.tsx` around lines 754 - 767, Update the comparison Y-axis domain calculation near chartData and maxComparisonValue to handle an empty chartData safely, using a finite non-negative fallback before applying Math.ceil. Preserve the existing scaling behavior when values are present and ensure comparisonYAxisDomain is always a valid Recharts domain.
🧹 Nitpick comments (1)
frontend/src/components/charts/AirQualityChart.tsx (1)
37-66: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getAqiPeriodBucket's "weekly" grouping doesn't matchgetReportBucket's weekly definition.
getReportBucketbuckets by calendar week (shifted to Monday), butgetAqiPeriodBucketbuckets by naive "week N of month" (Math.ceil(date.getUTCDate() / 7)), which doesn't align to weekday boundaries. On the same report page, the PM2.5 chart's "week" and the AQI category chart's "week" now mean different things, which is confusing when comparing the two.♻️ Suggested direction
const getAqiPeriodBucket = (timestamp: string, grouping: "monthly" | "weekly") => { - const date = new Date(timestamp) - const year = date.getUTCFullYear() - const month = date.getUTCMonth() - const monthLabel = date.toLocaleDateString("en-US", { month: "long", year: "numeric", timeZone: "UTC" }) - if (grouping === "monthly") { - ... - } - - const weekOfMonth = Math.ceil(date.getUTCDate() / 7) - return { - key: `${year}-${String(month + 1).padStart(2, "0")}-week-${weekOfMonth}`, - label: `Week ${weekOfMonth}, ${monthLabel}`, - sortValue: Date.UTC(year, month, (weekOfMonth - 1) * 7 + 1), - } + // reuse the same Monday-aligned weekly bucketing as getReportBucket for consistency }Also applies to: 68-87
🤖 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 `@frontend/src/components/charts/AirQualityChart.tsx` around lines 37 - 66, Align the weekly branch of getAqiPeriodBucket with getReportBucket by deriving each bucket from the calendar week’s Monday in UTC, rather than using Math.ceil(date.getUTCDate() / 7). Use matching Monday-based keys, labels, and sort values so AQI and PM2.5 weekly groupings represent the same periods; leave daily and monthly behavior unchanged.
🤖 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 `@frontend/src/components/reports/NexusDateRangePicker.tsx`:
- Around line 186-192: Update the range validation used by rangeError and
handleApply to detect when includeTime is enabled and the selected dates are the
same day but startTime is later than endTime. Return an appropriate range error
and prevent onApply from receiving an inverted range, while preserving the
existing missing-end-date and maximum-range checks.
In `@frontend/src/components/reports/ReportDataModal.tsx`:
- Around line 181-188: Wrap the Select callback in the aggregation control so
its plain string value is adapted to ReportDataOptions["frequency"] before
calling onAggregationChange. Update the Select element’s onValueChange prop and
preserve the existing daily, weekly, and monthly options.
---
Duplicate comments:
In `@frontend/src/components/charts/AirQualityChart.tsx`:
- Around line 754-767: Update the comparison Y-axis domain calculation near
chartData and maxComparisonValue to handle an empty chartData safely, using a
finite non-negative fallback before applying Math.ceil. Preserve the existing
scaling behavior when values are present and ensure comparisonYAxisDomain is
always a valid Recharts domain.
---
Nitpick comments:
In `@frontend/src/components/charts/AirQualityChart.tsx`:
- Around line 37-66: Align the weekly branch of getAqiPeriodBucket with
getReportBucket by deriving each bucket from the calendar week’s Monday in UTC,
rather than using Math.ceil(date.getUTCDate() / 7). Use matching Monday-based
keys, labels, and sort values so AQI and PM2.5 weekly groupings represent the
same periods; leave daily and monthly behavior 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b61670b-6f20-40f0-9b07-056111686b89
📒 Files selected for processing (6)
frontend/src/app/reports/page.tsxfrontend/src/components/charts/AirQualityChart.tsxfrontend/src/components/reports/NexusDateRangePicker.tsxfrontend/src/components/reports/ReportDataModal.tsxfrontend/src/lib/types.tsfrontend/src/services/apiService.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/lib/types.ts
- frontend/src/services/apiService.tsx
- frontend/src/app/reports/page.tsx
| <Select value={aggregation} onValueChange={onAggregationChange}> | ||
| <SelectTrigger><SelectValue /></SelectTrigger> | ||
| <SelectContent> | ||
| <SelectItem value="daily">Day</SelectItem> | ||
| <SelectItem value="weekly">Week</SelectItem> | ||
| <SelectItem value="monthly">Month</SelectItem> | ||
| </SelectContent> | ||
| </Select> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does assigning a function with a narrower union parameter type to a callback prop typed (value: string) => void cause a TypeScript strictFunctionTypes error?
💡 Result:
Yes, enabling strictFunctionTypes causes a TypeScript error when assigning a function with a narrower parameter type to a callback prop typed with a broader parameter type [1][2]. Under default TypeScript settings, function parameters are checked bivariantly, which allows assigning a function with a narrower parameter (e.g., string) to a type expecting a broader one (e.g., string | number) [3][2]. This behavior is considered unsound because the callback may be invoked with a value (e.g., a number) that the narrower function implementation is not designed to handle [1][4]. When strictFunctionTypes is enabled (or when using strict mode), TypeScript enforces contravariant parameter checking for non-method function types [1][5]. Under these stricter rules, the assignment is rejected because the source function type is not a subtype of the target function type [1][6]. Note that this stricter checking applies to functions defined using function syntax (including callback parameters), but does not apply to functions defined using method syntax in interfaces or classes, which remain bivariant even when strictFunctionTypes is enabled [1][7].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/strictFunctionTypes.html
- 2: https://stackoverflow.com/questions/53447348/type-checking-of-function-assignment-functions-with-union-types-arguments
- 3: https://www.typescriptlang.org/docs/handbook/type-compatibility.html
- 4: https://stackoverflow.com/questions/51767338/what-is-the-benefit-of-using-strictfunctiontypes-in-typescript
- 5: Strict function types microsoft/TypeScript#18654
- 6: https://stackoverflow.com/questions/57357165/typescript-functions-are-assignable-but-produces-error-if-target-parameter-type
- 7: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-6.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)(ReportDataModal|select)\.tsx$|(^|/)tsconfig|package\.json$|pnpm-lock|package-lock|yarn.lock' | sed -n '1,120p'
echo
echo "== ReportDataModal relevant lines =="
if [ -f frontend/src/components/reports/ReportDataModal.tsx ]; then
nl -ba frontend/src/components/reports/ReportDataModal.tsx | sed -n '1,240p'
fi
echo
echo "== select wrapper lines =="
if [ -f frontend/src/ui/select.tsx ]; then
nl -ba frontend/src/ui/select.tsx | sed -n '1,230p'
fi
echo
echo "== tsconfig references =="
for f in $(git ls-files | rg '(^|/)tsconfig.*\.json$|(^|/)package\.json$|(^|/)pnpm-lock\.yaml$|(^|/)package-lock\.json$|(^|/)yarn\.lock$'); do
case "$f" in
*frontend*) echo "--- $f" ;;
*package.json*) echo "--- $f" ;;
*tsconfig*.json*) echo "--- $f" ;;
esac
sed -n '1,220p' "$f" | rg -n '"extends"|"rootDir"|"noEmit"|"strictFunctionTypes"|"strict"|"strictNullChecks"|"composite"|"typescript"|"radix-ui|Select' || true
done
echo
echo "== all onValueChange SelectItem/value change patterns in frontend/src =="
rg -n "SelectItem value=|onValueChange=|use.*Change|select.tsx" frontend/src || trueRepository: airqo-platform/code-samples
Length of output: 940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ReportDataModal relevant lines =="
awk '{printf "%6d: %s\n", NR, $0}' frontend/src/components/reports/ReportDataModal.tsx | sed -n '90,210p'
echo
echo "== select wrapper lines =="
awk '{printf "%6d: %s\n", NR, $0}' frontend/src/ui/select.tsx | sed -n '1,220p'
echo
echo "== frontend/tsconfig.json =="
awk '{printf "%6d: %s\n", NR, $0}' frontend/tsconfig.json
echo
echo "== package TypeScript/Radix versions =="
node - <<'JS'
const fs = require('fs');
[
'frontend/package.json',
'frontend/package-lock.json'
].forEach(file => {
const pkg = JSON.parse(fs.readFileSync(file, 'utf8'));
console.log('---', file);
try {
if (file.endsWith('package.json')) {
console.log(JSON.stringify(pkg.dependencies || {}, null, 2));
console.log(JSON.stringify(pkg.devDependencies || {}, null, 2));
} else {
const pkgs = {};
for (const [k,v] of Object.entries(pkg.packages || {})) {
if (k === 'node_modules/typescript' || k.includes('node_modules/@radix-ui/react-select') || k.includes('node_modules/@radix-ui/react-popover') || k.includes('node_modules/@radix-ui/react-primitive')) {
pkgs[k] = v.version || pkg.version;
}
}
console.log(JSON.stringify(pkgs, null, 2));
}
} catch (e) {
console.log('read_parse_error', e.toString());
}
});
JS
echo
echo "== select callback occurrences =="
rg -n "onValueChange|SelectItem value=|onDataTypeChange|onAggregationChange" frontend/src/components frontend/src/ui || trueRepository: airqo-platform/code-samples
Length of output: 23457
🌐 Web query:
TypeScript strictFunctionTypes function literal parameter variance error assigning callback with narrower parameter type
💡 Result:
When the strictFunctionTypes compiler option is enabled in TypeScript, the language enforces contravariant checking for function parameters, which correctly prevents unsafe assignments [1][2][3]. Understanding the Error In default TypeScript mode, function parameters are checked bivariantly, meaning a function (x: SubType) => void is considered assignable to (x: SuperType) => void, and vice-versa [2][4]. While this is technically unsound, it is permitted to support common JavaScript patterns [4][5]. With strictFunctionTypes enabled, TypeScript switches to strict contravariant checking for function parameters [2][3]. Under these rules, a function type (a: A) => void is assignable to (a: B) => void if and only if B is a subtype of A (i.e., A is a supertype of B) [6][7]. If you attempt to assign a callback with a narrower parameter type (e.g., expecting a Dog) to a variable or location expecting a wider parameter type (e.g., accepting any Animal), TypeScript will correctly flag this as an error because it is unsafe [1][6]. The runtime could pass an Animal that is not a Dog to the callback, causing a crash or unexpected behavior when the code attempts to access Dog-specific properties [1][8]. Key Nuances and Exceptions 1. Method Syntax Exception: To prevent breaking common patterns in generic interfaces and classes (like Array), the strictFunctionTypes flag only applies to functions defined via function type syntax (e.g., (x: T) => void) [1][2][3]. Functions defined using method syntax (e.g., { func(x: T): void }) remain bivariant even when strictFunctionTypes is enabled [1][2]. 2. Callback Parameters: When using strictFunctionTypes, callback parameters are also checked strictly (contravariantly) to ensure type safety in higher-order functions [9]. To resolve these errors, you generally need to ensure your function parameter types are as general as the interface requires, or use generics to accurately define the relationship between the expected and provided types [6].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/strictFunctionTypes.html
- 2: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-6.html
- 3: Strict function types microsoft/TypeScript#18654
- 4: https://www.typescriptlang.org/docs/handbook/type-compatibility.html
- 5: Docs: function parameter bivariance microsoft/TypeScript#14973
- 6: https://stackoverflow.com/questions/69200228/error-when-replacing-unknown-with-more-specific-type-in-function-parameters
- 7: https://sammart.in/post/2021-07-25-typescript-function-type-parameter-contravariance/
- 8: https://stackoverflow.com/questions/78939508/why-the-functions-contravariance-with-their-paramaters-in-typescript
- 9: Strictly check callback parameters microsoft/TypeScript#18976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ReportDataModal top declarations and remaining relevant lines =="
awk '{printf "%6d: %s\n", NR, $0}' frontend/src/components/reports/ReportDataModal.tsx | sed -n '1,90p'
awk '{printf "%6d: %s\n", NR, $0}' frontend/src/components/reports/ReportDataModal.tsx | sed -n '210,245p'
echo
echo "== package manager / lockfile TS/Radix exact entries for frontend =="
node - <<'JS'
const fs = require('fs');
const lock = JSON.parse(fs.readFileSync('frontend/package-lock.json', 'utf8'));
const packages = lock.packages || {};
const keys = ['/typescript', '/@radix-ui/react-select', '/@radix-ui/react-popover', '/@radix-ui/react-primitive'];
for (const key of keys) {
const pkg = packages[key];
console.log(key, JSON.stringify(pkg && {version: pkg.version, dependencies: pkg.dependencies, peerDependencies: pkg.peerDependencies}.version || null));
}
JS
echo
echo "== lockfile `@types` entries relevant to Radix/typescript **if present** =="
rg -n '"`@types/react`"|"node_modules/@types/react"|node_modules/react-select|typescript' frontend/package-lock.json | head -n 80 || trueRepository: airqo-platform/code-samples
Length of output: 9707
Wrap the aggregation callback for Select.onValueChange.
Select inherits Radix’s plain string onValueChange, while onAggregationChange only accepts "daily" | "weekly" | "monthly". Use an adapter like onValueChange={(value) => onAggregationChange(value as ReportDataOptions["frequency"])} to avoid a strictFunctionTypes assignment error under the existing frontend/tsconfig.json.
🤖 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 `@frontend/src/components/reports/ReportDataModal.tsx` around lines 181 - 188,
Wrap the Select callback in the aggregation control so its plain string value is
adapted to ReportDataOptions["frequency"] before calling onAggregationChange.
Update the Select element’s onValueChange prop and preserve the existing daily,
weekly, and monthly options.
1 participant
feat: create ReportDataModal component for generating site reports
refactor: update types for site data and report options
fix: enhance API service for fetching site report data
Summary by CodeRabbit