Modernize fdm-app with oxlint/oxfmt and improve type safety#363
Modernize fdm-app with oxlint/oxfmt and improve type safety#363SvenVw wants to merge 26 commits into
Conversation
…ulator and fdm-app are type-safe
🦋 Changeset detectedLatest commit: 5981cf5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds oxfmt/oxlint tooling and CI workflows, tightens TypeScript typings across fdm-app/fdm-core/fdm-calculator, adjusts many form resolver typings, removes some component props/usages (e.g., NitrogenBalanceChart.balance), updates tests and repository configs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #363 +/- ##
===============================================
+ Coverage 87.62% 92.12% +4.49%
===============================================
Files 79 8 -71
Lines 3959 165 -3794
Branches 1145 101 -1044
===============================================
- Hits 3469 152 -3317
+ Misses 490 13 -477
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…n permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
…n permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
coderabbitai
Bot
changed the title
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
fdm-app/app/components/blocks/soil/cards.tsx (1)
18-39: Remove unusedtitleprop from type annotation.The
titleprop is declared in the type signature at line 29 but is not destructured from the function parameters. Althoughtitleis passed when calling this component at line 179, it is never used. This inconsistency suggests incomplete refactoring.Apply this diff to remove the unused prop from the type annotation:
function SoilDataCard({ shortname, value, label, unit, type, link, date, source, sourceLabel, }: { - title: string shortname: string value: number | string label: string | undefined unit: string type: "numeric" | "enum" link: string date: Date source: string sourceLabel: string }) {And update the component invocation to stop passing the unused prop:
return ( <SoilDataCard key={card.title} - title={card.title} shortname={card.shortname} value={card.value} label={card.label}fdm-core/src/field.test.ts (1)
940-947: Critical: Fix incorrectaddHarvestcall causing test failure.The test is failing because
addHarvestis being called with incorrect parameters. According to the pipeline error and the harvest test files, the function expects an object with harvest parameters, not a plain number.For HC010 harvest category, the required parameter is
b_lu_yield_fresh(not a plain value).Apply this diff to fix the test:
const harvestId = await addHarvest( fdm, principal_id, cultivationId, new Date("2025-08-10"), - // @ts-expect-error - 5000, + { + b_lu_yield_fresh: 10000, + b_lu_dm: 200, + b_lu_n_harvestable: 15, + }, )The parameters should match the HC010 harvest category requirements as seen in the harvest test file (lines 722-731).
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx (1)
155-162: Avoid returning rawerrorobject indataWithWarningPassing the caught
Errordirectly as the data parameter todataWithWarning(error, "Shapefile is ongeldig.")is inconsistent with the codebase pattern. All otherdataWithWarningcalls pass structured data objects (e.g.,{}or{warning: ...}). Raw error objects serialize unpredictably and can expose unintended details to the client; the warning message is sufficient.Keep error handling simple by logging server-side and passing minimal data:
- } catch (error) { - return dataWithWarning(error, "Shapefile is ongeldig.") + } catch (error) { + console.error("Shapefile parsing failed", error) + return dataWithWarning({}, "Shapefile is ongeldig.") }fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (2)
123-127: Cleanup effect with dependencies will run during component lifecycle, not just on unmount.This effect appears to be intended as a cleanup-only effect (it only has a cleanup function). However, adding
form.resetandformto the dependency array (line 127) means the cleanup will run whenever these dependencies change during the component's lifecycle, not just on unmount. This causesform.reset()to be called unexpectedly during normal operation.For cleanup-only effects, the dependency array should be empty
[].Apply this diff to fix the cleanup effect:
- }, [form.reset, form]) + }, [])If the intent is to reset the form whenever something changes (not just on unmount), the effect should have a regular effect body, not just a cleanup function.
103-112: Remove redundantformfrom dependency arrays.The effect at lines 103-112 includes both
form.resetand the entireformobject in its dependency array. Sinceform.resetis already a method reference from theformobject, including the whole object is redundant. TheuseRemixFormhook returns a stable form reference that doesn't change during the component lifecycle, soform.resetalone is sufficient.Additionally, the cleanup-only effect at lines 123-127 should have an empty dependency array since it only needs to run on unmount. Currently it has
[form.reset, form], which could cause the cleanup to run unnecessarily if the form reference changes.Update both dependency arrays:
- }, [uploadState, form.reset, form]) + }, [uploadState, form.reset])- }, [form.reset, form]) + }, [])fdm-app/app/routes/organization.$slug.tsx (1)
309-313:cancel_invitesuccess message usesformValues.emailwhich is never postedIn the
cancel_invitebranch, the success message interpolatesformValues.email, but the corresponding form (InvitationRow) only postsinvitation_id. This will renderUitnodiging voor undefined is ingetrokken.Either:
- Include the email in the form and schema:
<form method="post"> <input type="hidden" name="invitation_id" value={invitation.invitation_id} /> + <input + type="hidden" + name="email" + value={invitation.email} + />or
- Change the success message to not depend on
Also applies to: 457-467
fdm-core/src/organization.test.ts (1)
34-96: Investigate beforeAll hook timeout.The pipeline reports a timeout in the
beforeAllhook at line 34. This suggests the authentication setup or user creation is taking longer than expected (>10 seconds).The timeout could be caused by:
- Database connection issues in the test environment
- Hanging promises in
fdmAuth.api.signUpEmail- The type change to
anypotentially masking an error that's being swallowedRun the following to check for similar timeout patterns and async handling:
#!/bin/bash # Check for other test timeouts and beforeAll patterns echo "=== Checking test timeout configurations ===" rg -n "timeout|beforeAll" --type ts -g "**/*.test.ts" -C 2 echo -e "\n=== Checking fdmAuth.api usage patterns ===" rg -n "fdmAuth\.api\." --type ts -g "**/*.test.ts" -C 3Consider:
- Adding explicit timeout handling in beforeAll
- Verifying the test database is properly initialized
- Adding error logging in the catch block at line 92-95
fdm-app/app/components/blocks/rotation/table.tsx (1)
203-209:selectedCultivationsuseMemo won't recompute whenrowSelectionchanges iftablereference stays stableThe
useMemodepends only ontable, butuseReactTablemay return a stable instance even whenrowSelectionupdates in the state object. This meansselectedCultivationswon't recalculate on selection changes, leavingisFertilizerButtonDisabled,isHarvestButtonDisabled, and related logic stuck at stale values.Add
rowSelectionto the dependency array:- }, [table]) + }, [table, rowSelection])fdm-app/app/components/blocks/nutrient-advice/skeletons.tsx (1)
32-47:NutrientCardSkeletonRepeatnow always renders a single skeleton (array wrapping bug).
return [Array.from({ length: count })].map(...)creates an array with a single element (the inner array), so you only render oneNutrientCardSkeletonregardless ofcount. Call sites that passprimaryNutrients.length, etc., will no longer get the expected number of placeholders.Fix by mapping directly over the array of length
count:-export function NutrientCardSkeletonRepeat({ count }: { count: number }) { - return [Array.from({ length: count })].map((_, i) => ( +export function NutrientCardSkeletonRepeat({ count }: { count: number }) { + return Array.from({ length: count }).map((_, i) => ( // biome-ignore lint/suspicious/noArrayIndexKey: all items are the same <NutrientCardSkeleton key={i} /> )) }Also applies to: 65-69
♻️ Duplicate comments (4)
fdm-core/src/farm.test.ts (1)
37-37: Type safety regression (duplicate concern).Same type safety concern as flagged in organization.test.ts - the
fdmAuthvariable is now typed asanyinstead of a proper type.fdm-core/src/principal.test.ts (1)
3-3: Type safety regression (duplicate concern).Same type safety concern as flagged in organization.test.ts and farm.test.ts - the removal of typed imports and using
anyforfdmAuthreduces compile-time type safety.Also applies to: 19-19
fdm-core/src/authorization.test.ts (1)
3-3: Type safety regression (duplicate concern).Same type safety concern as flagged in previous test files - using
anyforfdmAutheliminates compile-time type checking.Also applies to: 29-29
fdm-core/src/authentication.test.ts (1)
16-16: Type safety regression (duplicate concern).Same type safety concern as in other test files -
fdmAuthnow typed asany.
🧹 Nitpick comments (24)
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.cultivations.$b_lu_catalogue.crop.harvest.new.tsx (1)
168-169: Consider improving type safety in parameter checks.The code uses
as Record<string, any>casts to access form values. Since this PR modernizes type safety across the codebase, consider extracting a typed helper or narrowing the form values type to eliminate the cast and improve IDE support.Also applies to: 192-195
fdm-app/app/components/blocks/field/popup.tsx (1)
45-50: Avoid rendering a dangling" ha"whenb_areais missingThe move to optional chaining is good for safety, but with:
<DialogDescription> {field.properties?.b_area} {" ha"} </DialogDescription>you’ll still render
" ha"even whenb_areaisundefined, resulting in a blank value with a unit.Consider rendering the block only when
b_areais present (and optionally doing the same forb_lu_nameif you don’t want an empty description):- <DialogDescription> - {field.properties?.b_area} - {" ha"} - </DialogDescription> + {field.properties?.b_area != null && ( + <DialogDescription> + {field.properties?.b_area} ha + </DialogDescription> + )}You could mirror this pattern for
b_lu_nameif an empty line is undesirable.fdm-app/app/routes/farm.create.$b_id_farm.$calendar.access.tsx (1)
49-83: Destructuring change is fine; consider cleaning up unusedcalendarin the loader.Dropping
calendarfrom theuseLoaderDatadestructuring is safe and matches the current component usage. If no other route or component reads this loader’s data viauseRouteLoaderData, you can simplify the loader by removinggetCalendar(params)and thecalendarfield from the returned object to avoid unused data and extra computation.Also applies to: 93-94
fdm-core/src/fdm-local.ts (1)
7-7: Good flexibility improvement—consider adding documentation.The relaxed typing from a literal
"memory://"to an inferredstringtype allows the function to accept file paths for persistent storage in addition to the in-memory default, which aligns well with PGlite's capabilities.However, it would be helpful to document what values are valid for the
backendparameter.Consider adding JSDoc documentation:
+/** + * Creates a local FDM instance using PGlite. + * + * @param backend - The PGlite backend to use. Can be "memory://" for in-memory storage, + * or a file path (e.g., "./data") for persistent storage. + * @returns The FDM instance providing the connection to the database. + */ export function createFdmLocal(backend = "memory://"): FdmLocalType {Alternatively, you could make the type explicit for clarity:
-export function createFdmLocal(backend = "memory://"): FdmLocalType { +export function createFdmLocal(backend: string = "memory://"): FdmLocalType {fdm-app/app/components/blocks/field/form.tsx (1)
44-52: Consider usingundefinedinstead of""forb_lu_cataloguedefaultYou currently coerce
b_lu_catalogueto an empty string:const b_lu_catalogue = field.properties?.b_lu_catalogue ?? ""Elsewhere in fdm-app, Select/Combobox fields prefer
undefinedover""as a fallback to avoid submitting empty strings as real values. You might want to align this with that pattern unless this dialog intentionally needs the empty string.Based on learnings, this project generally avoids
""as a fallback for select-like fields.fdm-app/app/routes/organization.new.tsx (1)
81-89: Consider adding explicit generic typing for consistency.While
zodResolverinfers types automatically, adding an explicit generic parameter would align with the PR's pattern of explicit Resolver typing seen in other route components.- const form = useRemixForm({ + const form = useRemixForm<z.infer<typeof FormSchema>>({ mode: "onTouched", resolver: zodResolver(FormSchema),fdm-core/src/field.ts (1)
81-92: Consider throwing an error for invalidb_acquiring_methodvalues.The current implementation logs a warning and silently converts invalid values to "unknown". While this is defensive, it may mask data quality issues. Consider throwing a validation error instead to force callers to provide valid values, especially since this function is inside a transaction and called after permission checks.
Alternatively, if silent conversion is intentional for backward compatibility, consider documenting this behavior in the function's JSDoc.
Apply this diff if you prefer strict validation:
- let validated_b_acquiring_method = b_acquiring_method - if ( - validated_b_acquiring_method && - !schema.acquiringMethodOptions - .map((option) => option.value) - .includes(validated_b_acquiring_method) - ) { - console.warn( - `Invalid b_acquiring_method: ${validated_b_acquiring_method}. Inserting as 'unknown'.`, - ) - validated_b_acquiring_method = "unknown" - } + if ( + b_acquiring_method && + !schema.acquiringMethodOptions + .map((option) => option.value) + .includes(b_acquiring_method) + ) { + throw new Error( + `Invalid b_acquiring_method: ${b_acquiring_method}. Must be one of: ${schema.acquiringMethodOptions.map(o => o.value).join(', ')}` + ) + }And update the insertion to use the original value:
- b_acquiring_method: validated_b_acquiring_method, + b_acquiring_method: b_acquiring_method,fdm-core/src/db/schema-custom-types.ts (1)
152-159: LGTM: localOffset correctly tracks position.The introduction of
localOffsetprevents mutation of the input parameter and clearly tracks the read position. The formula approach (localOffset + i * 16) correctly calculates each point's offset after the 4-bytenumPointsfield.Optional: Consider consistency with
readPolygon.
readPolygonuses an incremental approach (localOffset += 16in the loop) while this uses a formula approach. Both are correct, but matching styles would improve consistency. The formula approach here is actually cleaner, so you might consider applying it toreadPolygonas well in a future refactor.fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx (1)
107-107: Avoidanywhen extendingfieldResultwitherrorIdLines 107 and 115 relax type-safety with
as any. Runtime is fine, but this undermines the broader type‑safety push in this PR.Instead of
any, consider introducing an explicit extended type and using that in the loader result:// near the top of the file import type { OrganicMatterBalanceFieldResultNumeric } from "@svenvw/fdm-calculator" type FieldResultWithErrorId = OrganicMatterBalanceFieldResultNumeric & { errorId?: string }Then narrow the casts to that type:
- fieldResult = { ...fieldResult, errorId } as any + fieldResult = { ...fieldResult, errorId } as FieldResultWithErrorId- return { - fieldResult: fieldResult as any, - fieldInput: inputForField, - } + return { + fieldResult: fieldResult as FieldResultWithErrorId, + fieldInput: inputForField, + }This keeps the added
errorIdproperty while preserving strong typing for the rest of the calculator result.Also applies to: 115-115
fdm-app/app/components/custom/dropzone.tsx (1)
210-218: Ref forwarding logic is solid; adjust typings to avoid implicit globalReactand consider forwardRefThe new
refcallback correctly supports both function refs and object refs while keepinginputRefin sync. Two small follow‑ups:
- Typing:
DropzonePropsusesReact.Ref<HTMLInputElement>and the cast usesReact.MutableRefObject, butReactis not imported as a type namespace. Under typical TS configs this will error. You can avoid the globalReactreference by reusing existing imports, for example:-import type { InputHTMLAttributes, ReactNode } from "react" +import type { + InputHTMLAttributes, + ReactNode, + MutableRefObject, +} from "react" export type DropzoneProps = { - ref?: React.Ref<HTMLInputElement> + ref?: InputHTMLAttributes<HTMLInputElement>["ref"] // ... } // ... -} else if (ref) { - ;(ref as React.MutableRefObject<HTMLInputElement | null>).current = node -} +} else if (ref) { + ;(ref as MutableRefObject<HTMLInputElement | null>).current = node +}
- Semantics (optional):
Dropzoneis not usingforwardRef, so JSX<Dropzone ref={...} />will not actually passrefthrough as a normal prop. Long‑term, consider convertingDropzonetoforwardRef<HTMLInputElement, DropzonePropsWithoutRef>and using the forwarded ref inside this callback, instead of declaringrefinDropzoneProps. This would make ref behavior align with standard React expectations.fdm-app/app/routes/organization.$slug.tsx (1)
162-228: Optional: DRY up duplicatedmember/permissionsinline types
MemberRowandMemberActionrepeat the same inlinememberandpermissionsshapes. This works but increases maintenance overhead when roles/fields change.Consider extracting small shared types:
type OrganizationMember = { firstname: string surname: string username: string role: string image: string } type OrganizationPermissions = { canEdit: boolean canDelete: boolean canInvite: boolean canUpdateRoleUser: boolean canRemoveUser: boolean } const MemberRow = ({ member, permissions }: { member: OrganizationMember permissions: OrganizationPermissions }) => { /* ... */ } const MemberAction = ({ member, permissions }: { member: OrganizationMember permissions: OrganizationPermissions }) => { /* ... */ }This keeps prop typing consistent and easier to evolve.
fdm-core/src/organization.test.ts (1)
3-3: Type safety regression in test variable.The removal of the
BetterAuthtype import and changingfdmAuthfrom a typed variable toanyreduces type safety in the test suite. While this pattern appears consistently across other test files (authorization.test.ts, principal.test.ts, farm.test.ts), usinganyeliminates compile-time type checking for the auth object.Consider whether this type loosening is necessary. If the issue is with importing the
BetterAuthtype, you could:
- Export and use the return type of
createFdmAuthdirectly- Use
typeof createFdmAuthwithReturnType<>utility type- Document why
anyis needed if there's a specific reasonExample alternative:
let fdmAuth: ReturnType<typeof createFdmAuth>Also applies to: 32-32
fdm-app/app/components/blocks/rotation/table.tsx (1)
166-169: Fuzzy search target cast looks good; optional: tighten the data type to avoid castingThe new
(row.original as unknown as { searchTarget: string }).searchTargetcast is a clear improvement overanyand accurately reflects howmemoizedDatais built. If you want to avoid casting entirely, you could also constrainTDatahere to something likeTData extends RotationExtended & { searchTarget: string }, but that’s purely optional.fdm-app/app/components/blocks/fields/table.tsx (1)
126-131: Type-safe fuzzy filter cast is fine; optional generic tighteningUsing
(row.original as unknown as { searchTarget: string }).searchTargetremoves the
anyhole and matches howmemoizedDatais constructed, so this is a solid improvement. If you’d like to eliminate the cast, you could instead constrain the component generic to includesearchTarget(e.g.TData extends FieldExtended & { searchTarget: string }>), but that’s purely a nice-to-have.fdm-app/app/components/blocks/nutrient-advice/sections.tsx (1)
11-16: Type-safety improvement with room for further refinement.Replacing
anywithRecord<string, any>fornutrientAdvicedocuments the expected structure better. The explicitDosetype for the doses property significantly improves type-safety.If the structure of
nutrientAdviceis known, consider defining a more specific interface instead ofRecord<string, any>to eliminate the remaininganyusage entirely.fdm-app/app/components/blocks/nutrient-advice/cards.tsx (1)
34-40: Dose typing fordoses.doselooks correct; consider mirroring numeric guards from KPI for future-proofing.Switching
doses.dosetoDosealigns this component with@svenvw/fdm-calculatorand keeps thedoseTotalcalculation consistent with other nutrient-advice components. GivenDose’s broader index signature, you might optionally mirror the KPI’stypeof value === "number"-style guard fornumberOfApplicationsForNutrientto make the intent explicit ifdoseParameterwere ever to point at a non-numeric key, but the current behavior is unchanged and domain-knowledge-driven. Based on learnings, this standardizes on the shared Dose model.Also applies to: 62-64
.github/workflows/typecheck.yml (1)
1-66: Workflow wiring for type-checking across packages looks solid; consider adding PR trigger if you want gating.The job scopes
GITHUB_TOKENviapermissions: contents: read, uses pnpm + Turbo caching, and runs per-package build/typecheck in the expected working directories — this is a clean setup and resolves the earlier “workflow does not contain permissions” warning. If you want type errors to gate reviews rather than only post-merge, you could additionally hook this workflow topull_request(or a subset of branches).package.json (1)
23-29: New lint/format scripts are coherent; consider cleaning up any remaining Biome-specific config if it’s fully retired.The
lint/lint:fix+format/format:check+check/fixscript set is consistent and gives a clear local and CI story around oxlint/oxfmt. That said,pnpm.onlyBuiltDependenciesstill lists@biomejs/biome; if none of the workspaces use Biome anymore, you can optionally drop that entry (and any other Biome-specific remnants) to avoid confusion about which tooling is active.Also applies to: 30-35, 53-62
fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1)
15-21: AdoptingDoseplus numeric guards tightens KPI calculations; verify that all counted fields are truly “nutrients”.Switching all three KPI props to
Doseand guarding withtypeof value === "number"(and(doses.dose[doseParameter] as number | undefined) ?? 0) is a good fit for the shared calculator model and avoids accidentally treating string/undefined properties as applied nutrients. One thing to double-check is that every numeric field on the aggregatedDoseyou pass intoNutrientKPICardForTotalApplicationsactually represents a nutrient amount; otherwise,numberOfNutrientsAppliedwill include those extra numeric metadata fields in its count. If that’s not desired, you may want to restrict the count to known nutrient keys instead ofObject.values. Based on learnings, this keeps KPI logic aligned with the central Dose definition.Also applies to: 32-35, 76-83, 100-106, 190-196, 210-218
fdm-app/app/components/blocks/atlas-fields/query.tsx (1)
3-9: Stronger GeoJSON typing; consider guarding on geometry typeSwitching from an untyped cast to
Feature<Polygon | MultiPolygon>is a good step toward type safety and better expresses the turf expectation. If there’s any chance the FGB stream might contain non‑polygon geometries in future, you could optionally add a runtime guard onfeature.geometry.typebefore callingbooleanPointInPolygonto fail fast instead of relying solely on data contracts.Also applies to: 33-39
fdm-app/app/components/blocks/soil/form.tsx (1)
190-200: Residualanytype cast on DatePicker name prop.The
name={x.parameter as any}cast persists despite the type-safety improvements elsewhere. This occurs because the DatePicker'snameprop likely expects a union of specific field names, not a dynamic string.Consider creating a type-safe mapping or updating the DatePicker component's name prop to accept the dynamic parameter type.
fdm-app/app/components/blocks/harvest/form.tsx (1)
77-101: Multipleanycasts in resolver wrapper reduce type-safety gains.The resolver wrapper uses
anyfor all parameters and the final return type, which undermines the type-safety improvements this PR aims to achieve:resolver: (async (values: any, bypass: any, options: any) => { // ... }) as any,Additionally, the return on line 98 uses
{ values: {}, errors: true }which is non-standard. Theerrorsproperty typically expects an object with field-specific errors, not a boolean.Consider using proper types for the resolver parameters:
-resolver: (async (values: any, bypass: any, options: any) => { +resolver: (async ( + values: z.infer<typeof FormSchema>, + context: unknown, + options: { criteriaMode?: string; fields?: Record<string, unknown>; shouldUseNativeValidation?: boolean } +) => {For the error return, verify this format is intentional for blocking submission without displaying errors.
fdm-app/app/components/blocks/atlas/atlas-geocoder.tsx (2)
51-62: Inconsistent handling between maptiler and OSM providers.The maptiler path returns
datadirectly (line 61), while the OSM path maps results toGeocoderResult[]. This creates inconsistent return types.Additionally,
config.queryis typed asstring | number[], butString(config.query)on an array would produce"lon,lat"which may not be the intended geocoding query format.Consider:
- Mapping maptiler results similarly to ensure consistent
{ features: GeocoderResult[] }return format- Narrowing
querytype to juststringifnumber[]isn't a valid use caseinterface ForwardGeocodeConfig { - query?: string | number[] + query?: string limit?: number }
112-114: Double type assertion suggests incomplete type conformance.The
as unknown as GeocoderResultpattern indicates the constructed object doesn't fully match theGeocoderResultinterface. The object includes extra properties (place_name,place_type,text) that aren't in theGeocoderResulttype (which only extendsGeoJSONFeaturewithcenterandbbox).Consider extending the
GeocoderResultinterface to include these additional properties for proper type coverage:interface GeocoderResult extends GeoJSONFeature { center: [number, number] bbox?: [number, number, number, number] + place_name?: string + place_type?: string[] + text?: string }
| const applicationMethods = fertilizerParameterDescription.find( | ||
| (x) => x.parameter === "p_app_method_options", | ||
| (x: any) => x.parameter === "p_app_method_options", | ||
| ) | ||
| if (!applicationMethods) throw new Error("Parameter metadata missing") | ||
| // Map fertilizers to options for the combobox | ||
| const fertilizerOptions = fertilizers.map((fertilizer) => { | ||
| const applicationMethodOptions = fertilizer.p_app_method_options | ||
| .map((opt) => { | ||
| .map((opt: any) => { | ||
| const meta = applicationMethods.options.find( | ||
| (x) => x.value === opt, | ||
| (x: any) => x.value === opt, | ||
| ) | ||
| return meta ? { value: opt, label: meta.label } : undefined | ||
| }) | ||
| .filter( | ||
| (option): option is { value: string; label: string } => | ||
| (option: any): option is { value: string; label: string } => | ||
| option !== undefined, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace explicit any types with proper types from fdm-core.
The PR aims for full type-safety, but using explicit any annotations defeats this purpose. The correct types are available from the fdm-core schema.
Apply proper typing based on the fdm-core schema:
const fertilizerParameterDescription =
getFertilizerParametersDescription()
const applicationMethods = fertilizerParameterDescription.find(
- (x: any) => x.parameter === "p_app_method_options",
+ (x) => x.parameter === "p_app_method_options",
)
if (!applicationMethods) throw new Error("Parameter metadata missing")
// Map fertilizers to options for the combobox
const fertilizerOptions = fertilizers.map((fertilizer) => {
const applicationMethodOptions = fertilizer.p_app_method_options
- .map((opt: any) => {
+ .map((opt) => {
const meta = applicationMethods.options.find(
- (x: any) => x.value === opt,
+ (x) => x.value === opt,
)
return meta ? { value: opt, label: meta.label } : undefined
})
.filter(
- (option: any): option is { value: string; label: string } =>
+ (option): option is { value: string; label: string } =>
option !== undefined,
)TypeScript inference should handle these types correctly based on the schema definitions in fdm-core.
🤖 Prompt for AI Agents
In fdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsx
around lines 143 to 159, replace the explicit any annotations with the
corresponding types from fdm-core (e.g., the parameter description type used for
fertilizerParameterDescription, the fertilizer record/type for items in
fertilizers, and the option shape for p_app_method_options); import the
appropriate types from fdm-core at the top of the file, type applicationMethods
as the parameter description with an options array, type
fertilizer.p_app_method_options as an array of the option value type, and
replace the filter/type-guard with a typed predicate that narrows to { value:
string; label: string } so TypeScript inference removes the any usage and
preserves the same runtime logic.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
fdm-app/app/routes/organization.$slug.tsx (1)
414-419: Previousas anyissue resolved withExtendedUsercastCasting
session.usertoExtendedUserhere matches the expected shape forrenderInvitationEmailand removes the unsafeas anythat was flagged earlier. This brings the action in line with the rest of the auth/user typing.fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (1)
146-159: Prefer the concrete soil data item type (or inference) over{ parameter: string }infindpredicates.
getCurrentSoilDataalready returns a strongly-typed array; annotating the predicate parameter as{ parameter: string }throws away the richer type information and duplicates what TypeScript can infer. It’s also the same spot previously flagged for usingany, so the underlying concern (using the real soil item type) remains.Consider either:
- Importing and using the concrete type from
fdm-core(e.g.CurrentSoilDataItem) for both predicates, or- Dropping the explicit annotation and letting TS infer the correct item type from
currentSoilData.For example:
- const a_som_loi = - currentSoilData.find( - (x: { parameter: string }) => x.parameter === "a_som_loi", - )?.value ?? null + const a_som_loi = + currentSoilData.find( + (x) => x.parameter === "a_som_loi", + )?.value ?? null @@ - const b_soiltype_agr = - currentSoilData.find( - (x: { parameter: string }) => x.parameter === "b_soiltype_agr", - )?.value ?? null + const b_soiltype_agr = + currentSoilData.find( + (x) => x.parameter === "b_soiltype_agr", + )?.value ?? nullIf you prefer explicit typing, replace
(x)with(x: CurrentSoilDataItem)(or the equivalent inline interface) instead.
🧹 Nitpick comments (4)
fdm-calculator/src/norms/index.test.ts (1)
75-80: TS error suppression correctly scoped inside arrow functionPlacing
// @ts-expect-errorimmediately before the invalid call inside the arrow function cleanly targets just that line while still asserting the runtime"Year not supported"error. This is a clear and type-safe way to cover the negative case.If you want to improve future readability, you could annotate one of these with a short reason, e.g.:
// @ts-expect-error: verify runtime error for unsupported year "2024"to make the intent explicit to maintainers.
fdm-core/src/field.test.ts (1)
752-774: Extended cultivation catalogue payload matches enriched modelAdding
b_lu_harvestcat,b_lu_dm,b_lu_eom, andb_lu_eom_residuesto theaddCultivationToCataloguefixture keeps this test in sync with the expanded cultivation model and should prevent silent drift between schema and tests. The chosen values are reasonable for a fixture and don’t introduce edge cases.If you find yourself duplicating this catalogue fixture across multiple tests, consider extracting a small helper factory to centralize defaults and reduce maintenance effort.
fdm-app/app/routes/logout.tsx (1)
24-28: Subtle behavior change:nullvsundefinedfor revokeSession tokenBy dropping the
?? undefinedcoalesce,tokenwill now benullifsession.session.tokenisnull, whereas previously it normalizednulltoundefined(and would typically be omitted from JSON payloads if stringified). Ifauth.api.revokeSessionor the downstream API expects either a string or an omitted field (not explicitnull), this could change behavior.If
session.session.tokencan ever benull, consider keeping the previous normalization:token: session?.session?.token ?? undefined,or otherwise confirm the revoke endpoint explicitly accepts
nullhere.fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (1)
107-115: Alignb_areahandling betweenfieldOptionsandfieldsExtended(nullish vs non-nullish).In
fieldOptionsyou useMath.round(field.b_area * 10) / 10, while infieldsExtendedyou now doMath.round((field.b_area ?? 0) * 10) / 10. Ifb_areacan ever be nullish, this difference could give youNaNinfieldOptionsbut0infieldsExtended; if it’s guaranteed non-null, the?? 0is unnecessary.Consider making both places consistent with whichever contract you want (either “required number” or “number with
0fallback”), for example:- return { - b_id: field.b_id, - b_name: field.b_name, - b_area: Math.round(field.b_area * 10) / 10, - } + return { + b_id: field.b_id, + b_name: field.b_name, + b_area: Math.round((field.b_area ?? 0) * 10) / 10, + } @@ - b_area: Math.round((field.b_area ?? 0 )* 10) / 10, + b_area: Math.round((field.b_area ?? 0) * 10) / 10,Also applies to: 170-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.oxlintrc.json(1 hunks).vscode/settings.json(1 hunks)fdm-app/.oxlintrc.json(1 hunks)fdm-app/app/components/blocks/balance/skeletons.tsx(1 hunks)fdm-app/app/components/blocks/fertilizer-applications/form.tsx(9 hunks)fdm-app/app/components/blocks/norms/skeletons.tsx(1 hunks)fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx(2 hunks)fdm-app/app/components/blocks/soil/list.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsx(1 hunks)fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id._index.tsx(3 hunks)fdm-app/app/routes/logout.tsx(1 hunks)fdm-app/app/routes/organization.$slug.tsx(2 hunks)fdm-app/app/routes/welcome.tsx(1 hunks)fdm-app/tsconfig.json(1 hunks)fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.ts(9 hunks)fdm-calculator/src/balance/organic-matter/degradation.test.ts(7 hunks)fdm-calculator/src/balance/organic-matter/index.test.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/cultivation.test.ts(2 hunks)fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts(6 hunks)fdm-calculator/src/balance/organic-matter/supply/index.test.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/residues.test.ts(1 hunks)fdm-calculator/src/doses/get-dose-field.test.ts(1 hunks)fdm-calculator/src/norms/index.test.ts(2 hunks)fdm-calculator/tsconfig.json(1 hunks)fdm-core/src/authentication.ts(2 hunks)fdm-core/src/field.test.ts(8 hunks)fdm-core/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- fdm-calculator/src/balance/organic-matter/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- fdm-app/app/components/blocks/norms/skeletons.tsx
- fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsx
- .oxlintrc.json
- fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id._index.tsx
🧰 Additional context used
🧠 Learnings (54)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/residues.test.tsfdm-calculator/src/doses/get-dose-field.test.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-calculator/src/balance/organic-matter/degradation.test.tsfdm-calculator/src/balance/organic-matter/supply/cultivation.test.tsfdm-calculator/src/norms/index.test.tsfdm-calculator/src/balance/organic-matter/supply/index.test.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.test.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/residues.test.tsfdm-calculator/src/doses/get-dose-field.test.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-calculator/src/balance/organic-matter/degradation.test.tsfdm-calculator/src/balance/organic-matter/supply/cultivation.test.tsfdm-calculator/src/norms/index.test.tsfdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-calculator/src/balance/organic-matter/supply/fertilizers.test.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Applied to files:
fdm-app/tsconfig.jsonfdm-core/tsconfig.jsonfdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-calculator/tsconfig.jsonfdm-app/.oxlintrc.jsonfdm-core/src/authentication.ts
📚 Learning: 2024-12-11T12:09:35.540Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the `fdm-app/tsconfig.json` file, the include path `.react-router/types/**/*` refers to a build-time generated directory which is intentionally not included in the repository.
Applied to files:
fdm-app/tsconfig.jsonfdm-core/tsconfig.jsonfdm-calculator/tsconfig.jsonfdm-app/.oxlintrc.json
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-app/tsconfig.jsonfdm-core/tsconfig.jsonfdm-calculator/tsconfig.jsonfdm-app/.oxlintrc.jsonfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2025-08-13T10:33:05.313Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
Applied to files:
fdm-app/tsconfig.jsonfdm-core/tsconfig.jsonfdm-calculator/tsconfig.jsonfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2024-11-27T09:58:24.047Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-core/src/cultivation.d.ts:3-13
Timestamp: 2024-11-27T09:58:24.047Z
Learning: In the `fdm-core` TypeScript codebase, interface names should not be prefixed with 'I'; interface names should be meaningful without the 'I' prefix.
Applied to files:
fdm-app/tsconfig.jsonfdm-calculator/tsconfig.jsonfdm-core/src/authentication.ts
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/tsconfig.jsonfdm-core/tsconfig.jsonfdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-calculator/tsconfig.jsonfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/authentication.tsfdm-core/src/field.test.ts
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-calculator/src/doses/get-dose-field.test.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-app/app/routes/organization.$slug.tsxfdm-core/src/authentication.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-app/app/routes/organization.$slug.tsxfdm-core/src/authentication.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-app/app/routes/organization.$slug.tsxfdm-core/src/authentication.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts currently returns only firstname, surname, image, and role, but needs to include id and email fields to support downstream operations like role updates and user removal. This will be fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-app/app/routes/organization.$slug.tsxfdm-core/src/authentication.ts
📚 Learning: 2025-04-18T14:20:40.975Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:70-76
Timestamp: 2025-04-18T14:20:40.975Z
Learning: The organization schema in fdm-core/src/db/schema-authn.ts is managed by better-auth, and modifications to field constraints (like making the slug field non-nullable) should maintain compatibility with better-auth's expectations, even if application code assumes non-null values.
Applied to files:
fdm-app/app/routes/organization.$slug.tsxfdm-core/src/authentication.ts
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.
Applied to files:
fdm-app/app/routes/organization.$slug.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
📚 Learning: 2024-11-27T11:27:27.797Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T11:27:27.797Z
Learning: When cleaning up test data in `afterAll` hooks in `fdm-data/src/cultivations/index.test.ts`, use `fdm.delete(schema.tableName).execute();` to delete rows from a table without dropping the table itself.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-calculator/src/balance/organic-matter/degradation.test.tsfdm-calculator/src/balance/organic-matter/supply/cultivation.test.ts
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-core/src/field.test.ts
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.tsfdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-11-21T10:02:25.556Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 343
File: fdm-calculator/src/balance/organic-matter/types.d.ts:12-132
Timestamp: 2025-11-21T10:02:25.556Z
Learning: In the organic matter balance calculation system (fdm-calculator), degradation values are negative by definition. This means the balance calculation using supply.total.plus(degradation.total) is correct, as it effectively computes supply - |degradation|. This follows the same pattern as the nitrogen balance system.
Applied to files:
fdm-calculator/src/balance/organic-matter/degradation.test.ts
📚 Learning: 2025-11-24T10:43:09.278Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 343
File: fdm-app/app/components/blocks/balance/organic-matter-chart.tsx:19-25
Timestamp: 2025-11-24T10:43:09.278Z
Learning: In the organic matter balance chart (fdm-app), degradation values are multiplied by -1 before plotting to show supply and degradation bars side-by-side with positive magnitudes, enabling direct visual comparison. This is a deliberate design choice for the organic matter chart visualization, different from the nitrogen balance chart which uses separate stacks.
Applied to files:
fdm-calculator/src/balance/organic-matter/degradation.test.ts
📚 Learning: 2025-08-11T11:57:42.225Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields.$centroid.tsx:0-0
Timestamp: 2025-08-11T11:57:42.225Z
Learning: In Remix loaders, the `getSession(request)` call should be retained even when the returned session value isn't used elsewhere in the function, as it serves as an authentication guard that validates the user has a valid session and likely throws an error or redirects if the session is invalid. This is a common security pattern to protect routes.
Applied to files:
fdm-app/app/routes/welcome.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/welcome.tsx
📚 Learning: 2025-02-24T10:49:54.523Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 84
File: fdm-app/app/root.tsx:89-145
Timestamp: 2025-02-24T10:49:54.523Z
Learning: In the ErrorBoundary component of fdm-app/app/root.tsx, all client errors (400, 401, 403, 404) are intentionally displayed with a 404 status code for security purposes.
Applied to files:
fdm-app/app/routes/welcome.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/welcome.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Applied to files:
fdm-app/app/routes/welcome.tsx
📚 Learning: 2025-07-21T12:06:07.070Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 156
File: fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts:295-303
Timestamp: 2025-07-21T12:06:07.070Z
Learning: Functions in the fdm-calculator with "NL2025" in their names are specifically designed for Netherlands 2025 agricultural norms calculation and hardcoded 2025 dates are appropriate in this context, as different years would have separate calculation modules.
Applied to files:
fdm-calculator/src/norms/index.test.ts
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2024-12-16T10:56:33.616Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/components/custom/combobox.tsx:35-37
Timestamp: 2024-12-16T10:56:33.616Z
Learning: When using `useRemixForm`, the correct type for the `form` prop is `UseRemixFormReturn<T>` from `remix-hook-form`, not `UseFormReturn` from `react-hook-form`.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-core/src/authentication.ts
📚 Learning: 2024-11-25T14:42:26.660Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2024-11-25T14:42:26.660Z
Learning: In `fdm-app/app/components/blocks/field-map.tsx`, explicit cleanup of Mapbox GL resources is not necessary, as `react-map-gl` handles it automatically upon component unmount, and `MapRef` does not have a `remove` method.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2024-12-16T11:28:58.089Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/components/custom/combobox.tsx:35-37
Timestamp: 2024-12-16T11:28:58.089Z
Learning: When using `UseRemixFormReturn` from `remix-hook-form`, it is not a generic type and should be used without type parameters.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2025-06-03T12:27:28.823Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 153
File: fdm-app/app/components/custom/soil/formschema.tsx:17-26
Timestamp: 2025-06-03T12:27:28.823Z
Learning: In fdm-app Zod validation schemas, required numeric fields use direct `z.coerce.number()` without preprocessing, while optional numeric fields use `z.preprocess((val) => (val === "" ? undefined : val), z.coerce.number().optional())` to convert empty strings to undefined. This pattern ensures required fields properly validate empty inputs as missing, while optional fields treat empty strings as truly optional.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/form.tsx
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/field.test.ts
📚 Learning: 2025-04-18T14:20:40.975Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:70-76
Timestamp: 2025-04-18T14:20:40.975Z
Learning: The schema defined in fdm-core/src/db/schema-authn.ts follows better-auth's structure and requirements. While the schema is defined in the application code, modifications to it should maintain compatibility with better-auth's expectations.
Applied to files:
fdm-core/src/authentication.ts
📚 Learning: 2025-04-18T14:18:44.350Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:69-76
Timestamp: 2025-04-18T14:18:44.350Z
Learning: The authentication schema (fdm-authn) is based on better-auth's schema structure, which does not include `updatedAt` fields for tables like `organization`.
Applied to files:
fdm-core/src/authentication.ts
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:1-1
Timestamp: 2025-01-14T16:06:24.294Z
Learning: In the fdm-app codebase, the `redirect` function should be imported from `react-router`, not `react-router-dom`.
Applied to files:
fdm-core/src/authentication.ts
📚 Learning: 2025-01-14T16:06:21.832Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm.settings._index.tsx:1-1
Timestamp: 2025-01-14T16:06:21.832Z
Learning: In the fdm project, `redirect` and other routing utilities should be imported from `react-router` instead of `react-router-dom`.
Applied to files:
fdm-core/src/authentication.ts
📚 Learning: 2025-03-04T11:09:08.169Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-core/src/cultivation.ts:246-246
Timestamp: 2025-03-04T11:09:08.169Z
Learning: In the FDM codebase, the `fdm` parameter should be documented as "The FDM instance providing the connection to the database. The instance can be created with {link createFdmServer}." in JSDoc comments.
Applied to files:
fdm-core/src/authentication.ts
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-core/src/field.test.ts
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-core/src/field.test.ts
📚 Learning: 2025-03-04T09:03:04.358Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 87
File: fdm-core/src/farm.ts:22-25
Timestamp: 2025-03-04T09:03:04.358Z
Learning: In the FDM codebase, functions that insert/create data (like `addFarm`) intentionally use `principal_id: string` instead of `PrincipalId` to enforce that only a single principal ID can be used for creation operations, while read operations use the more flexible `PrincipalId` type which supports both single IDs and arrays of IDs.
Applied to files:
fdm-core/src/field.test.ts
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-core/src/field.test.ts
🔇 Additional comments (45)
.vscode/settings.json (1)
2-4: Verify VS Code formatter alignment with oxfmt/oxlint tooling shift.The changes remove biome formatter settings but replace them with
vscode.typescript-language-features(VS Code's built-in TypeScript formatter) rather than configuring oxfmt. Since the PR objective is to modernize with oxfmt/oxlint, there's a potential mismatch: developers' IDE formatting may diverge from the project's oxfmt standards enforced in CI, causing formatting conflicts and friction during development.Additionally, the removal of
editor.formatOnSaveremoves an important developer safety net for automatic code formatting.Please clarify the intended formatting workflow:
- Should developers use VS Code's built-in formatter (current config), oxfmt (via extension if available), or neither?
- Is the removal of
formatOnSaveintentional, or should it be configured to use an oxfmt extension if available?- Are there planned oxfmt/oxlint VS Code extensions or configurations that should be added to this settings file?
If oxfmt is the canonical formatter, consider updating the settings to configure an oxfmt VS Code extension (if available) and re-enabling
editor.formatOnSaveto keep IDE and CI formatting in sync.fdm-app/app/routes/organization.$slug.tsx (1)
40-40: Good use ofimport typeforExtendedUserUsing a type-only import for
ExtendedUserkeeps this dependency compile‑time only and avoids affecting the runtime bundle. Looks clean and consistent with the type‑safety goals of the PR.fdm-app/.oxlintrc.json (3)
1-8: Configuration structure is sound.The JSON is well-formed, the extends pattern is appropriate for a monorepo, and the ignore patterns follow best practices. No syntax or structural issues detected.
1-8: Verify parent configuration file exists at the correct path.This configuration extends from
../../.oxlintrc.json(the root .oxlintrc.json). Please confirm this parent config file exists at the repository root and is properly configured.
3-7: Verify ignore patterns cover all fdm-app build outputs.The current patterns cover standard directories (
build/,dist/,node_modules/). Based on the build configuration,.react-router/types/**/*is generated at build time and may need to be added to the ignore patterns if it generates lint warnings during the build process. Confirm whether other framework-specific output directories (e.g., from Vite or Remix) should also be included.fdm-calculator/src/norms/index.test.ts (3)
37-42: Good use of@ts-expect-errorfor runtime negative testUsing
// @ts-expect-errorhere is appropriate to exercise the runtime error path for an unsupported year that the type system intentionally disallows. This keeps typings strict while still testing the guard logic.
44-49: Pattern consistently applied for unsupported region caseSame pattern here for an unsupported region looks correct: the type-level restriction is acknowledged via
@ts-expect-errorand the runtime behavior is asserted withtoThrow("Region not supported"). This matches the intended NL‑only/2025‑only design for these norm functions (based on learnings about NL2025 modules).
82-87: Consistent negative test for unsupported region in fertilizer fillingThe final negative test mirrors the earlier ones and keeps the contract clear: types only allow valid NL/2025 combinations, while tests still assert the
"Region not supported"runtime behavior via@ts-expect-error. Looks solid.fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.ts (4)
31-80: Type safety improvements look good.The changes from
undefinedto explicitnullforp_ef_nh3and the correction ofp_typetonullimprove type strictness and align with the PR's modernization goals. The distinction between "no predefined emission factor" (null) and undefined is now explicit.
82-97: Explicit boolean values improve test clarity.Changing
m_cropresiduefromundefinedto explicittrue/falsevalues makes the test data more explicit and type-safe, clearly expressing the intent for each cultivation scenario.
684-684: Proper use of type error suppression for negative tests.The
@ts-expect-errorannotations correctly document intentional type violations in tests that verify error handling for unsupported application methods. This is the recommended pattern for negative test cases.Also applies to: 709-709, 734-734
332-332: Good: Maintaining consistency in commented code.Updating the commented test data to use
nullensures future consistency when thep_inhibitorfeature is implemented.fdm-core/src/authentication.ts (2)
9-9: Type-only import forFdmAuthis correct and matches its usage.Keeping
FdmAuthas a type-only import aligns with it being a purely compile-time type alias and avoids any unnecessary runtime import.Please re-run the type-check / auth tests to confirm there are no remaining value-usage sites for
FdmAuth.
89-219: Leaning on inference forauthwhile keeping the function return type explicit is a good balance.
authis inferred directly frombetterAuth(...), and the explicitcreateFdmAuth(...): FdmAuthreturn type still guarantees compatibility with the exported alias, so there’s no loss of type safety here.After this refactor, ensure the auth-related tests still compile and pass so any mismatch between
FdmAuthandbetterAuthwould be caught.fdm-app/app/components/blocks/fertilizer-applications/form.tsx (11)
1-39: LGTM!The import of
Resolvertype fromreact-hook-formis appropriate for the resolver typing added later in the form initialization.
60-67: LGTM!The inline type definition for
fertilizerApplicationis explicit and supports both single (p_app_id) and batch (p_app_ids) modes while correctly making the entire prop optional for add/edit scenarios.
72-89: LGTM!The
Resolver<FieldFertilizerFormValues>cast onzodResolveris appropriate here to satisfy TypeScript's type requirements. The defaultValues correctly handle the optionalfertilizerApplicationprop, and deferringp_app_amountinitialization to an effect is a valid pattern for inputs requiring special blank behavior.
94-102: LGTM!The oxlint-disable comment correctly addresses the stable reference concern. The conditional logic properly resets
p_app_methodonly when the fertilizer selection actually changes.
114-123: LGTM!The hydration logic with explicit type casting for
FieldFertilizerFormValueskeys improves type safety. The date string-to-Date conversion handles serialized store values correctly.
135-140: LGTM!This effect correctly handles the deferred
p_app_amountinitialization as noted in the defaultValues comment.
142-149: LGTM!The effect correctly updates the fertilizer selection when the user returns from adding a new fertilizer.
151-161: LGTM!The cleanup effect correctly removes stored form data after successful submission, preventing stale data from persisting.
163-174: LGTM!The function correctly saves form state before navigating to the new fertilizer page, enabling seamless return with preserved data.
177-177: LGTM!Removing the
as anycast from theRemixFormProviderspread is a good type safety improvement.
263-273: LGTM!The NaN handling is thorough and correctly uses
Number.isNaN(not the globalisNaN) to avoid type coercion issues. The defensive approach handles edge cases wherefield.valuecould be in various states.fdm-core/tsconfig.json (1)
18-22: LGTM! Monorepo path alias configuration is well-structured.The addition of
baseUrland the@svenvw/fdm-datapath alias enables consistent module resolution across the monorepo, allowing fdm-core to import directly from fdm-data source files during development.fdm-calculator/tsconfig.json (1)
20-24: LGTM! Path alias configuration aligns with monorepo structure.The
baseUrlchange and new path mappings for@svenvw/fdm-coreand@svenvw/fdm-dataenable fdm-calculator to import directly from sibling packages, consistent with the project's monorepo architecture.fdm-app/tsconfig.json (1)
32-35: LGTM! Comprehensive path alias setup for fdm-app.The three new path aliases enable fdm-app to import from fdm-core, fdm-calculator, and fdm-data using consistent module specifiers, completing the monorepo module resolution setup across all packages.
fdm-calculator/src/balance/organic-matter/supply/residues.test.ts (1)
7-7: LGTM! Import path normalization is appropriate.The change from
../types.dto../typesfollows TypeScript best practices by importing from the module itself rather than explicitly referencing the declaration file.fdm-calculator/src/balance/organic-matter/supply/cultivation.test.ts (2)
3-3: LGTM! Import path normalization is consistent.The import path change aligns with the broader refactoring to standardize type imports across test files.
21-21: Verify the crop rotation value change is intentional.The test data changes
b_lu_croprotationfrom"grassland"to"grass". Please confirm this aligns with updated schema definitions or standardized enum values throughout the codebase.fdm-calculator/src/balance/organic-matter/supply/index.test.ts (1)
8-8: LGTM! Import path normalization is consistent.The standardized import path aligns with the project-wide refactoring to normalize type imports.
fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts (3)
3-3: LGTM! Import path normalization is consistent.The import path change follows the project-wide standardization of type imports.
44-44: LGTM! Field name standardization top_app_amountis consistent.The change from
p_amounttop_app_amountacross all test cases indicates a field name standardization in the fertilizer application schema, improving clarity by explicitly indicating this is an application amount.Also applies to: 61-63, 92-92, 117-117
28-28: Verify thep_type: nullchange aligns with type definitions.The fertilizer type changed from
"other"tonullfor the "other-1" entry. Confirm that theFertilizerDetailtype definition allowsp_typeto benulland that this correctly represents fertilizers that don't fall into manure, compost, or mineral categories.fdm-calculator/src/doses/get-dose-field.test.ts (1)
105-112: Verify the updated fertilizer schema with the function signature.The test adds several new properties to the fertilizer catalogue entry (
p_app_method_options,p_no3_rt,p_nh4_rt,p_cr_rt,p_cr_vi,p_ef_nh3,p_type,p_type_rvo). Ensure thataddFertilizerToCataloguein fdm-core has been updated to accept these new properties in its type definition.fdm-calculator/src/balance/organic-matter/degradation.test.ts (4)
8-8: LGTM! Correct TypeScript import practice.Removing the
.dextension from type imports follows TypeScript best practices and likely aligns with the oxlint/oxfmt modernization mentioned in the PR objectives.
40-41: Type safety improvement with explicit field addition.The addition of
b_soiltype_agr: nullexplicitly satisfies stricter type requirements, which aligns with the PR's type safety objectives. The removal of the unit comment on line 40 is a minor documentation trade-off.
65-65: Consistent type safety improvements across all test cases.All test cases now explicitly include
b_soiltype_agr: null, satisfying stricter type requirements. The consistent use ofnullacross all test scenarios suggests this field is not utilized in the degradation calculation logic being tested.Also applies to: 89-89, 111-111, 137-137
21-21: Verify the crop rotation value change is intentional.The mock data for grass cultivation changed
b_lu_croprotationfrom"grassland"to"grass". Please verify this aligns with theCultivationDetaildata model and is used consistently across tests and the codebase.fdm-core/src/field.test.ts (2)
182-220: ConsistentPolygontyping for field geometries looks goodAll the
field*Geometryliterals being explicitly typed asPolygonkeeps the tests aligned withschema.fieldsTypeInsert["b_geometry"]and makes future refactors around geometry safer without changing runtime behavior. No further changes needed here.Also applies to: 258-269, 287-298, 313-324, 341-352
940-947: Structured yield object inaddHarvestcascade test is appropriateSwitching the
addHarvestcall to pass a yield object{ b_lu_yield_fresh: 5000 }aligns this test with the updated API and richer yield representation, while keeping the test focused on cascade deletion behavior rather than numeric yield details. This looks correct and future‑proof for additional yield fields.Based on learnings, this also fits the broader move towards more explicit yield modeling instead of overloading single numeric values.
fdm-app/app/routes/welcome.tsx (1)
295-305: Good defensive session guard in actionThe explicit
if (!session)check beforeupdateUserProfileis a solid addition: it aligns with the pattern of usinggetSession(request)as an auth guard in the loader while still handling the case where the session might be missing or expired between loader and action. ReturninghandleActionError(new Error("No session found"))keeps error handling consistent with the rest of the route. Based on learnings, this matches the project’s authentication guard conventions.fdm-app/app/components/blocks/soil/list.tsx (2)
1-22: Type-safety changes aroundSoilAnalysesListprops look solid.The switch to
FetcherWithComponents<SoilActionData>and the dedicatedSoilActionDatatype keeps this component aligned with the PR’s objective of eliminatinganyand makes the fetcher contract explicit. Given that this component doesn’t currently consumefetcher.data, the setup is future‑proof without overcomplicating the signature.
42-45: Disabled edit link remains keyboard-accessible.Using
pointer-events-noneandaria-disableddoes not prevent keyboard users from tabbing to the link and activating it with Enter or Space. Per MDN and W3C accessibility guidelines,aria-disabledonly exposes state to assistive technology and does not automatically suppress activation—keyboard event handlers must be added to prevent Enter/Space activation. Additionally,pointer-events-nonehas no effect on keyboard navigation.Prevent keyboard activation by either conditionally rendering a non-interactive element (e.g.,
<span>) whenisDisabledis true, or by adding anonClickhandler that checksisDisabledand callsevent.preventDefault()alongsideonKeyDownguards for Enter/Space.Also applies to: 75-96
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
2 participants
Summary by CodeRabbit
New Features
Bug Fixes / UX
Chores
✏️ Tip: You can customize this high-level summary in your review settings.