Optimize build system: migrate to esbuild and standardize code#351
Conversation
- Automate `external` dependency detection from package.json to prevent accidentally bundling dependencies (fixes potential code
duplication).
- Disable `terser` minification during development to significantly improve rebuild speeds.
- Standardize source maps: use inline maps for development and external file maps for production.
Add `@rollup/plugin-commonjs` to `fdm-data` to ensure compatibility with CommonJS dependencies.
- Removing "vibe-based" manual chunk splitting, allowing Vite/Rollup to handle chunking more effectively.
- Simplifying the conditional Sentry plugin logic for better readability and maintainability.
- Removing the unnecessary `global: {}` polyfill, which is often a workaround for legacy libraries and can hide issues.
- Correcting the `sentryReactRouter` argument passing to resolve a TypeScript error.
🦋 Changeset detectedLatest commit: 3f1cbe0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
|
Caution Review failedThe pull request is closed. WalkthroughRefactors monorepo build and packaging across packages, replacing TypeScript/terser Rollup plugins with esbuild, standardizing preserveModules/sourcemaps, tightening Vite Sentry activation to env-based logic, normalizing type import paths, adding build-only tsconfigs, and making typing and dose-shape refinements. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Env as build env
participant Vite as vite.config
participant Sentry as sentryReactRouter
note over Vite,Sentry `#DDEBF7`: Sentry plugin enabled only when env.mode="production" and SENTRY_AUTH_TOKEN exists
Env->>Vite: load(env)
alt production + token
Vite->>Sentry: sentryReactRouter(..., env)
Sentry-->>Vite: pluginInstance
Vite-->>build: include pluginInstance
else otherwise
Vite-->>build: do not include Sentry
end
sequenceDiagram
autonumber
participant RollupCfg as rollup.config
participant Esbuild as rollup-plugin-esbuild
participant PKG as package.json
participant FS as src/
participant Dist as dist/
RollupCfg->>PKG: read dependencies/peerDependencies
RollupCfg->>FS: input src/index.ts
RollupCfg->>Esbuild: transform (minify if isProd)
RollupCfg->>RollupCfg: mark externals based on PKG + node builtins
RollupCfg->>Dist: output preserveModules -> dir with entryFileNames "[name].js" and sourcemaps (inline or true)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #351 +/- ##
============================================
Coverage 87.59% 87.60%
============================================
Files 79 79
Lines 3951 3952 +1
Branches 1141 1140 -1
============================================
+ Hits 3461 3462 +1
Misses 490 490
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:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 0
🧹 Nitpick comments (9)
fdm-core/src/field.ts (1)
493-496: Inline.mapparameter typings are correct but duplicate schema knowledgeThe new annotations
{ b_lu: string },{ b_id_harvestable: string },{ b_id_harvestable_analysis: string }, and{ a_id: string }look accurate, but they mirror column shapes manually. To keep types single‑sourced and future‑proof, consider either:
- Dropping the inline param types and let the
select({ ... })result drive inference, e.g.cultivations.map((c) => c.b_lu), or- Typing the ID arrays instead, anchored on the schema, e.g.:
const cultivationIds: schema.cultivationsTypeSelect["b_lu"][] = cultivations.map((c) => c.b_lu)Same idea for
harvestableIds,harvestableAnalysisIds, andsoilAnalysisIds.Also double‑check that the selected columns are non‑nullable in the schema; if any can be
null, the currentstringannotations would be too narrow and should be widened accordingly.Also applies to: 510-513, 530-533, 604-606
fdm-core/src/principal.ts (1)
292-297: Explicit{ id: string }typing onprincipals.mapis correct; consider centralizing the row typeThe new parameter type matches how
principalsis built (select({ id: ... })on user and organization), so this is type-safe and makes the map callback clearer. As a minor refinement, you could avoid repeating the inline shape by deriving a small type from the schema (or from the originalselect) and reusing it here, which keeps things in sync if the DB field type ever changes.Example (optional):
type PrincipalIdRow = { id: string } // or derived from schema const principalsDetails = await Promise.all( principals.map(async (principal: PrincipalIdRow) => { return getPrincipal(tx, principal.id) }), )Not required for this PR, just a small maintainability tweak you might consider later.
fdm-app/app/components/blocks/sidebar/farm.tsx (1)
119-121: Truncation is good; consider exposing full label via tooltip/titleAdding
truncatehere should help keep long farm names from breaking the sidebar layout. Consider also adding atitle={farmLinkDisplay}on this span (or parent) so users can still see the full name on hover, especially when names are frequently longer than the available width.fdm-app/vite.config.ts (1)
2-10: Sentry gating viaenv.mode+ token looks sound; just verify plugin expectationsUsing
env.mode === "production"plus!!process.env.SENTRY_AUTH_TOKENto decideenableSentryis a good guardrail so the plugin only runs for actual prod builds with credentials configured. TheenableSentry && sentryReactRouter({...}, env)+.filter(Boolean)pattern keeps the plugin list clean and avoids conditionals scattered around.Given this depends on the exact
@sentry/react-routerversion, please double-check that passing the Vite config env as the second argument is what your installed version expects, and that the chosen env vars (PUBLIC_SENTRY_*,SENTRY_AUTH_TOKEN,npm_package_version) match how you configure Sentry elsewhere in the app.If you want, I can also help factor the Sentry options object into a shared
const sentryConfig = { ... }so it can be reused (and optionally surfaced on the returned config) without changing behavior.Also applies to: 16-31
fdm-core/rollup.config.js (2)
4-15: Externals derived from package.json look good; considernode:specifiers if you ever use themBuilding
dependenciesfromdependencies+peerDependenciesand adding only explicit Node builtins keeps the externals list tight and matches the “no extra externals” guideline. One minor nuance:externalwill not treatnode:fs-style imports as external (only barefsorfs/...), so if you ever switch tonode:specifiers you may want to extend the predicate accordingly (e.g. also checkingid.startsWith("node:")). Based on learnings, this approach to externals is consistent with earlier Rollup configs in this repo.Also applies to: 49-49
20-24: Align Rollupsourcemapand esbuildsourceMapbehaviour across envsRight now, Rollup has
sourcemap: isProd ? true : "inline"while esbuild is configured withsourceMap: !isProd, so in production Rollup writes sourcemaps but esbuild does not emit TS-level maps, and in development you get inline maps from both. If you want full TS→JS mappings in production too, consider either dropping the explicitsourceMapoption and lettingrollup-plugin-esbuildfollow Rollup’soutput.sourcemap, or setting both to the same predicate (e.g.!!isProdor!isProd, depending on intent).Also applies to: 28-33
fdm-data/rollup.config.js (2)
4-12: Dependency‑driven externals are clean; consider subpaths if you ever import themBuilding
externaldirectly fromdependenciesandpeerDependencieskeeps the bundle surface minimal and adheres to the earlier guideline about only marking real deps as external. Note that with an array, only exact IDs (e.g."xxhash-wasm") are treated as external; if you later import subpaths (like"xxhash-wasm/foo"), you may want to switch to a predicate function similar tofdm-core/rollup.config.jsthat also matchesdep/....Also applies to: 33-34
15-22: Same sourcemap/esbuild inversion as fdm-core; consider unifying intentThis config also enables Rollup sourcemaps in production (
sourcemap: isProd ? true : "inline") while disabling esbuild’ssourceMapthere (sourceMap: !isProd), which can limit TS-level mapping in prod builds. If you want consistent behaviour, either letrollup-plugin-esbuildinfer source map usage from Rollup by omittingsourceMap, or set both to the same condition depending on whether you prefer dev-only or dev+prod sourcemaps.Also applies to: 27-31
fdm-calculator/src/doses/calculate-dose.ts (1)
133-133: Consider a type-safe alternative toas anycasting.The
as anycasting bypasses TypeScript's type safety for dynamic property access. While this works, it could mask future type errors.Consider one of these approaches:
Option 1: Create a helper type for numeric dose fields
type NumericDoseKeys = Exclude<keyof Dose, 'p_app_id'> for (const key of Object.keys(totalDose) as NumericDoseKeys[]) { if (key !== "p_app_id") { totalDose[key] += currentDose[key] } }Option 2: Use a type assertion that preserves some safety
for (const key of Object.keys(totalDose) as (keyof Dose)[]) { if (key !== "p_app_id") { (totalDose[key] as number) += (currentDose[key] as number) } }Either approach maintains better type safety while still handling the dynamic property access pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.changeset/jolly-eagles-clean.md(1 hunks).changeset/thin-baths-care.md(1 hunks)fdm-app/app/components/blocks/sidebar/farm.tsx(1 hunks)fdm-app/vite.config.ts(1 hunks)fdm-calculator/package.json(3 hunks)fdm-calculator/rollup.config.js(1 hunks)fdm-calculator/src/balance/nitrogen/index.ts(1 hunks)fdm-calculator/src/balance/organic-matter/degradation.ts(2 hunks)fdm-calculator/src/balance/organic-matter/index.ts(2 hunks)fdm-calculator/src/balance/organic-matter/input.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/cultivation.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/fertilizers.ts(2 hunks)fdm-calculator/src/balance/organic-matter/supply/index.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/residues.ts(1 hunks)fdm-calculator/src/balance/organic-matter/types.ts(0 hunks)fdm-calculator/src/balance/shared/soil.ts(1 hunks)fdm-calculator/src/doses/calculate-dose.ts(1 hunks)fdm-calculator/src/doses/get-dose-field.test.ts(2 hunks)fdm-calculator/src/doses/get-dose-field.ts(1 hunks)fdm-calculator/tsconfig.build.json(1 hunks)fdm-core/package.json(3 hunks)fdm-core/rollup.config.js(2 hunks)fdm-core/src/authorization.test.ts(1 hunks)fdm-core/src/db/schema-custom-types.ts(2 hunks)fdm-core/src/field.ts(4 hunks)fdm-core/src/principal.ts(1 hunks)fdm-core/src/soil.ts(1 hunks)fdm-core/tsconfig.build.json(1 hunks)fdm-data/package.json(3 hunks)fdm-data/rollup.config.js(1 hunks)fdm-data/tsconfig.build.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- fdm-calculator/src/balance/organic-matter/types.ts
🧰 Additional context used
🧠 Learnings (47)
📓 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`.
📚 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/supply/residues.tsfdm-calculator/src/balance/organic-matter/supply/index.tsfdm-calculator/src/balance/organic-matter/degradation.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/supply/residues.tsfdm-calculator/src/balance/organic-matter/index.tsfdm-calculator/src/balance/organic-matter/supply/index.tsfdm-calculator/src/balance/organic-matter/degradation.ts
📚 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 calculations, EOM (Effective Organic Matter) and OM (Organic Matter) are the same units and can be used together in calculations. EOM refers to the organic matter that remains in the soil after one year.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/residues.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/balance/organic-matter/supply/cultivation.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/components/blocks/sidebar/farm.tsx
📚 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: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/components/blocks/sidebar/farm.tsx
📚 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: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Applied to files:
fdm-app/app/components/blocks/sidebar/farm.tsx
📚 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 comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/components/blocks/sidebar/farm.tsx
📚 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:
.changeset/jolly-eagles-clean.md.changeset/thin-baths-care.mdfdm-data/tsconfig.build.jsonfdm-calculator/tsconfig.build.jsonfdm-core/tsconfig.build.jsonfdm-data/rollup.config.jsfdm-core/package.jsonfdm-data/package.jsonfdm-core/rollup.config.jsfdm-app/vite.config.tsfdm-calculator/rollup.config.js
📚 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:
.changeset/jolly-eagles-clean.mdfdm-calculator/tsconfig.build.jsonfdm-calculator/package.jsonfdm-core/package.jsonfdm-data/package.jsonfdm-calculator/rollup.config.js
📚 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:
.changeset/jolly-eagles-clean.mdfdm-data/tsconfig.build.jsonfdm-core/tsconfig.build.jsonfdm-calculator/package.jsonfdm-data/rollup.config.jsfdm-core/package.jsonfdm-data/package.jsonfdm-core/rollup.config.js
📚 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/organic-matter/supply/fertilizers.tsfdm-calculator/src/doses/get-dose-field.tsfdm-calculator/src/balance/organic-matter/index.tsfdm-calculator/src/balance/organic-matter/input.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-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/doses/get-dose-field.test.tsfdm-calculator/src/doses/calculate-dose.tsfdm-calculator/src/doses/get-dose-field.tsfdm-core/src/db/schema-custom-types.tsfdm-core/src/field.tsfdm-data/package.json
📚 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/fertilizers.tsfdm-calculator/src/balance/organic-matter/index.tsfdm-calculator/src/balance/organic-matter/supply/cultivation.tsfdm-core/src/field.tsfdm-calculator/src/balance/organic-matter/supply/index.tsfdm-calculator/src/balance/organic-matter/degradation.ts
📚 Learning: 2025-08-14T14:31:55.384Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 236
File: fdm-calculator/src/balance/nitrogen/index.ts:173-0
Timestamp: 2025-08-14T14:31:55.384Z
Learning: In nitrogen balance calculations for agricultural systems, the balance should only include ammonia emissions (emission.ammonia.total) and should not include nitrate leaching from the emission calculation. The nitrate component (emission.nitrate) should be excluded from the balance formula.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2025-05-26T10:32:15.538Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.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-core/src/authorization.test.tsfdm-core/src/principal.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-core/src/authorization.test.tsfdm-core/src/principal.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-core/src/authorization.test.tsfdm-core/src/principal.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-core/src/authorization.test.tsfdm-core/src/principal.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/authorization.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 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-core/src/authorization.test.ts
📚 Learning: 2025-02-14T09:56:37.606Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
Applied to files:
fdm-calculator/src/doses/get-dose-field.test.tsfdm-calculator/src/doses/calculate-dose.tsfdm-calculator/src/doses/get-dose-field.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-calculator/src/doses/get-dose-field.test.tsfdm-core/src/field.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-data/tsconfig.build.jsonfdm-calculator/tsconfig.build.jsonfdm-core/tsconfig.build.jsonfdm-core/package.jsonfdm-data/package.jsonfdm-core/rollup.config.jsfdm-app/vite.config.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: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-calculator/src/doses/get-dose-field.tsfdm-calculator/src/balance/organic-matter/input.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.tsfdm-calculator/src/balance/organic-matter/input.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-calculator/src/doses/get-dose-field.tsfdm-calculator/src/balance/organic-matter/index.tsfdm-calculator/src/balance/organic-matter/input.tsfdm-core/src/field.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-calculator/src/doses/get-dose-field.ts
📚 Learning: 2025-03-04T10:56:35.540Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-calculator/src/doses/calculate-dose.ts:18-18
Timestamp: 2025-03-04T10:56:35.540Z
Learning: In the FDM calculator, fertilizer nutrient rates (p_n_rt, p_p_rt, p_k_rt) are measured in g/kg, and are converted to fractions by dividing by 10 during dose calculations.
Applied to files:
fdm-calculator/src/doses/get-dose-field.ts
📚 Learning: 2025-05-28T07:57:19.217Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax `"typescript": "catalog:"` is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo. The catalog: syntax without a package name is the shorthand for referencing the default catalog.
Applied to files:
pnpm-workspace.yamlfdm-calculator/package.jsonfdm-core/package.jsonfdm-data/package.json
📚 Learning: 2025-05-28T07:57:19.217Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax `"typescript": "catalog:"` is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo.
Applied to files:
pnpm-workspace.yamlfdm-calculator/package.jsonfdm-core/package.jsonfdm-data/package.json
📚 Learning: 2025-01-24T11:46:49.990Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-data/rollup.config.js:7-17
Timestamp: 2025-01-24T11:46:49.990Z
Learning: When suggesting external dependencies in Rollup configuration, only include packages that are actually listed in the package's dependencies or peerDependencies.
Applied to files:
pnpm-workspace.yamlfdm-calculator/package.jsonfdm-data/rollup.config.jsfdm-core/package.jsonfdm-data/package.jsonfdm-core/rollup.config.jsfdm-calculator/rollup.config.js
📚 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/principal.ts
📚 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/cultivation.tsfdm-calculator/src/balance/organic-matter/degradation.ts
📚 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-core/src/field.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.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-core/src/field.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: 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-core/src/field.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.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-core/src/field.ts
📚 Learning: 2025-02-13T08:28:00.183Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-core/src/cultivation.ts:572-597
Timestamp: 2025-02-13T08:28:00.183Z
Learning: In the FDM database schema, `b_id_harvesting` is a primary key in the `cultivationHarvesting` table, ensuring no duplicate harvest entries can exist. Therefore, duplicate checking is not needed when processing harvest records from database queries.
Applied to files:
fdm-core/src/field.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-core/src/field.ts
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-01-24T11:39:57.805Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/cultivation.ts:625-671
Timestamp: 2025-01-24T11:39:57.805Z
Learning: Every cultivation has an associated `cultivationTerminating` record created by `addCultivation`, with `b_terminate_date` initially set to null. Therefore, `updateCultivation` can safely assume the record exists and only needs to handle updates.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-02-19T10:50:50.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 82
File: fdm-app/app/entry.client.tsx:13-13
Timestamp: 2025-02-19T10:50:50.200Z
Learning: Sentry DSN being public in client-side code is not a security concern as per Sentry documentation, but it can be made configurable using Vite's environment variables system with the `VITE_` prefix.
Applied to files:
fdm-app/vite.config.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/vite.config.ts
🧬 Code graph analysis (8)
fdm-core/src/authorization.test.ts (1)
fdm-core/src/authorization.ts (1)
roles(29-29)
fdm-calculator/src/doses/get-dose-field.ts (1)
fdm-calculator/src/doses/calculate-dose.ts (1)
calculateDose(37-142)
fdm-calculator/src/balance/organic-matter/index.ts (1)
fdm-calculator/src/balance/organic-matter/types.ts (2)
FertilizerDetail(235-240)CultivationDetail(224-230)
fdm-core/src/field.ts (1)
fdm-core/src/db/schema.ts (1)
cultivations(373-385)
fdm-calculator/src/balance/organic-matter/degradation.ts (1)
fdm-core/src/db/schema.ts (1)
cultivations(373-385)
fdm-data/rollup.config.js (2)
fdm-core/rollup.config.js (2)
isProd(7-7)external(15-15)fdm-calculator/rollup.config.js (2)
isProd(6-6)external(8-11)
fdm-app/vite.config.ts (3)
fdm-core/rollup.config.js (1)
isProd(7-7)fdm-calculator/rollup.config.js (1)
isProd(6-6)fdm-data/rollup.config.js (1)
isProd(7-7)
fdm-calculator/rollup.config.js (2)
fdm-core/rollup.config.js (2)
isProd(7-7)external(15-15)fdm-data/rollup.config.js (2)
isProd(7-7)external(9-12)
🔇 Additional comments (30)
.changeset/jolly-eagles-clean.md (2)
1-5: Valid changeset format with appropriate version bump level.The YAML frontmatter correctly declares patch-level version bumps across the three affected packages. This is appropriate for bug fixes (dependency bundling) and performance improvements.
7-11: Changeset description aligns well with PR objectives.The bullet points clearly summarize the build optimization work: fixing unintended dependency bundling, skipping minification in dev builds, and standardizing source maps.
However, verify whether
fdm-appshould be included in this changeset. The AI summary indicates that fdm-app also receives build optimizations (Vite config simplification, Sentry logic changes). If fdm-app is a user-facing or dependency package, it may warrant its own patch entry to inform consumers of changes.fdm-calculator/tsconfig.build.json (1)
1-9: Well-structured build configuration.This new build-specific TypeScript configuration appropriately extends the base tsconfig and excludes test-related files from the build scope. The pattern aligns with the PR's build optimization objective and follows standard TypeScript configuration best practices for isolating production builds.
fdm-calculator/src/balance/organic-matter/degradation.ts (1)
7-7: LGTM! Import path standardization and type clarity improvements.The changes improve code quality:
- Import path updated to follow TypeScript conventions (removing
.dextension)- Explicit type annotation on the callback parameter makes the code more self-documenting
These align with the broader PR effort to standardize type imports across the organic-matter module.
Also applies to: 43-43
fdm-calculator/src/balance/shared/soil.ts (1)
9-9: LGTM! Import path standardization.The import path update aligns with TypeScript conventions and is consistent with similar changes across the organic-matter module.
fdm-calculator/src/balance/organic-matter/input.ts (1)
16-16: LGTM! Import path standardization.The type import path update follows TypeScript conventions and aligns with the broader refactoring across the organic-matter module.
fdm-calculator/src/balance/organic-matter/supply/cultivation.ts (1)
6-6: LGTM! Import path standardization.The import path update removes the
.dextension and simplifies the path structure, aligning with TypeScript conventions and the broader module refactoring.fdm-calculator/src/balance/organic-matter/supply/index.ts (1)
7-7: LGTM! Import path standardization.The type import path update is consistent with the standardization effort across the organic-matter module.
fdm-calculator/src/balance/organic-matter/supply/fertilizers.ts (1)
6-6: LGTM! Import standardization and enhanced type safety.The changes improve code quality:
- Import path updated to follow TypeScript conventions
- Explicit type annotations in the
reducecallback enhance type safety and make the code more maintainableThese align with the broader refactoring to standardize types across the organic-matter module.
Also applies to: 37-37
fdm-calculator/src/balance/organic-matter/supply/residues.ts (1)
7-7: LGTM! Import path standardization.The type import path update is consistent with the standardization effort across the organic-matter module.
fdm-calculator/src/balance/organic-matter/index.ts (1)
18-18: LGTM! Comprehensive type safety improvements.The changes significantly enhance type clarity:
- Import path standardized to follow TypeScript conventions
- Explicit generic type parameters on
Mapdeclarations improve type inference- Typed callbacks in
mapfunctions make the code more maintainable and self-documentingThese changes are part of the broader effort to strengthen type safety across the organic-matter balance module.
Also applies to: 42-46
fdm-app/vite.config.ts (1)
37-42:optimizeDeps.excludealigns dev pre-bundling with your library build outputsExcluding
@svenvw/fdm-core,@svenvw/fdm-data, and@svenvw/fdm-calculatorfrom Vite’s dependency optimization is a good fit for a monorepo where those are built separately with Rollup. This should reduce duplicate bundling work and avoid dev-server issues if those packages ship modern ESM.No changes needed from my side here.
If you hit any dev-only issues after this (e.g., missing dependency pre-bundle warnings), it’d be worth confirming the published formats/entrypoints of these three packages match what Vite expects in your current version.
.changeset/thin-baths-care.md (1)
1-10: Changeset accurately summarizes the Vite/Sentry tweaksThe patch description matches the actual
vite.config.tschanges (simplified Sentry wiring, no manual chunks, noglobalpolyfill) and is scoped correctly as a patch for@svenvw/fdm-app. Looks good to commit.fdm-core/src/db/schema-custom-types.ts (2)
110-116: SRID value reading was intentionally removed and poses no risk to the codebase.The search reveals that the commented-out SRID reading was never actually used anywhere. The codebase hardcodes SRID 4326 in the geometry schema definition (line 43 of the same file:
geometry(${type},4326)), and coordinate system conversions use EPSG:4326 as a fixed target projection. The SRID value from binary data was being read but never stored or referenced by any downstream code. The offset handling and flag clearing remain correct, so the change is safe and represents legitimate cleanup of dead code.
32-32: Now I have all the information needed. Let me finalize the verification:The
geometryfunction is not exported in the public API. It's only defined inschema-custom-types.tsand used internally inschema.ts. The public API exportsfdmSchemawhich contains the table definitions, but not thegeometryfunction itself. Since consumers cannot directly call thegeometryfunction with different type parameters, this type narrowing is an internal implementation detail, not a breaking change to the public API.All internal uses of
geometry()in the codebase (found inschema.tsat lines 97 and 781) only use"Polygon"and"MultiPoint"types, which are now the only supported types after the change. The implementation inparseGeometryalso only handles these two types.
fdm-core/src/authorization.test.ts (1)
780-782: LGTM! Order-independent assertion improves test robustness.The updated assertion correctly verifies that both roles are present without enforcing a specific order, which is appropriate since the function queries the database and order may not be deterministic.
fdm-core/tsconfig.build.json (1)
1-10: LGTM! Clean build-time configuration.The build-specific TypeScript config appropriately excludes test files and utilities from production builds, following the standard pattern established across the monorepo.
fdm-calculator/src/balance/nitrogen/index.ts (1)
23-23: LGTM! Import reordering.The import statement has been relocated but remains functionally identical.
fdm-core/src/soil.ts (1)
547-547: LGTM! Explicit type annotation improves clarity.The
Record<string, any>annotation appropriately reflects the dynamic property access pattern used in this predicate.fdm-calculator/rollup.config.js (2)
3-29: LGTM! Clean migration to esbuild-based build pipeline.The configuration updates align well with the parallel changes in fdm-core and fdm-data, establishing a consistent build approach across packages. The switch to esbuild provides faster transpilation, and preserveModules maintains a clean module structure in the output.
Also applies to: 57-57
36-39: The placeholder replacement mechanism is correctly implemented and will work as intended.The implementation is sound:
Placeholder location verified: The placeholder
"fdm-calculator:{FDM_CALCULATOR_VERSION}"exists infdm-calculator/src/package.ts(line 2), which is part of the module tree and will be included in the build.Version replacement works correctly: The renderChunk plugin (lines 31-55) searches for the placeholder in all output chunks, replaces it with the actual version from
package.json(line 4), and pads the replacement to match the placeholder length to preserve source maps (line 50).Graceful handling with preserveModules: Returning
nullwhen the placeholder is not found (line 38) is the correct behavior—only the chunk containing the placeholder will be modified, while all other chunks remain untouched.Edge case handling: The code includes a warning (lines 41-45) if the replacement string would exceed the placeholder length, protecting source map integrity.
No issues found. The version replacement will correctly apply to the intended entry file through its exports.
pnpm-workspace.yaml (1)
14-14: LGTM! Workspace catalog aligned with esbuild migration.The updates appropriately introduce
rollup-plugin-esbuildto support the new build pipeline and bring vitest tooling to the latest patch versions.Also applies to: 19-19, 26-26
fdm-data/tsconfig.build.json (1)
1-9: LGTM! Build configuration mirrors fdm-core pattern.The build-specific TypeScript config appropriately excludes test files and utilities from production builds, consistent with the approach in fdm-core.
fdm-calculator/package.json (1)
15-16: LGTM! Package configuration aligned with new build output.The entry point updates (main, module, exports) correctly reflect the new
dist/index.jsoutput from the preserveModules build strategy, and the extended build script ensures TypeScript declarations are generated using the build-specific configuration.Also applies to: 22-22, 34-34, 56-56
fdm-core/package.json (1)
19-21: Entry points and build pipeline look consistent with Rollup config; double‑check tsconfig.build emit modePointing
main,module, and the export’sdefaultto./dist/index.jsmatches the Rollup output (input: "src/index.ts",preserveModules: true,entryFileNames: "[name].js"). Thebuildscript then runsrollup -c && tsc -p tsconfig.build.json; just ensuretsconfig.build.jsonis configured to emit only declarations (or at least not overwritedist/index.js) so Rollup remains the source of truth for JS.Also applies to: 26-26, 39-39, 58-58
fdm-data/package.json (1)
19-20: fdm-data package.json is aligned with the new Rollup/esbuild pipeline; verify TS build only emits d.tsThe
main/module/exportsdefault now all target./dist/index.js, which lines up with the new Rollup config (src/index.ts+preserveModules). Thebuildscript mirrors fdm-core; as there too, confirm thattsconfig.build.jsonis set up (e.g.emitDeclarationOnly) so TypeScript only contributes.d.tsfiles intodistand doesn’t overwrite the Rollup-generated JS bundle.Also applies to: 26-26, 37-37, 52-52
fdm-calculator/src/doses/get-dose-field.test.ts (2)
128-148: LGTM! Test expectations align with the simplified API.The test expectations correctly reflect the new return type from
getDoseForField, which now returns a flatDoseobject instead of a nested structure.
181-201: LGTM! Consistent test update for the zero-dose case.The test expectations are correctly updated to match the simplified return type.
fdm-calculator/src/doses/get-dose-field.ts (2)
54-54: LGTM! Proper error type casting.The error casting to
Errortype is the correct approach for TypeScript error handling, ensuring type-safe access to themessageproperty.
50-51: Based on my verification, I can now generate the rewritten review comment.No production callers found for the simplified return type change.
Verification shows that
getDoseForFieldis a newly added function (per CHANGELOG: "AddcalculateDoseandgetDoseForFieldto retrieve the nutrient doses") with no production code callers in the monorepo. The only callers are in the test file, which correctly expect and validate the simplified return type (onlyDoseobject, not{dose, applications}). The function is exported as public API but is not used elsewhere in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fdm-calculator/rollup.config.js (1)
4-4: Verify Node.js version compatibility with import assertions.The
with { type: "json" }syntax requires Node.js ≥20.10. This is the same concern as flagged in fdm-data/rollup.config.js.
🧹 Nitpick comments (1)
fdm-calculator/src/doses/d.ts (1)
40-42: Consider tightening the index signature for better type safety.The index signature
[key: string]: string | number | undefinedallows assigningundefinedto properties thatDosePropertiesdeclares as required numbers (e.g.,p_dose_n). While TypeScript permits this, it weakens type guarantees and may lead to runtime issues if consumers assume dose values are always present.If dynamic property access is needed for iteration (as suggested by the
NumericDoseKeysusage incalculate-dose.ts), consider alternatives:
- Use
Record<string, unknown>with type guards for dynamic keys- Or restrict the index signature to exclude DoseProperties keys:
[key: string]: unknownwith a more specific type helperExample refinement:
export interface Dose extends DoseProperties { - [key: string]: string | number | undefined + [key: string]: unknown }Then use type assertions or guards when accessing dynamic properties.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
fdm-calculator/rollup.config.js(1 hunks)fdm-calculator/src/doses/calculate-dose.ts(2 hunks)fdm-calculator/src/doses/d.ts(2 hunks)fdm-core/rollup.config.js(2 hunks)fdm-core/src/field.ts(4 hunks)fdm-data/rollup.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- fdm-core/src/field.ts
- fdm-calculator/src/doses/calculate-dose.ts
- fdm-core/rollup.config.js
🧰 Additional context used
🧠 Learnings (4)
📓 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`.
📚 Learning: 2025-01-24T11:46:49.990Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-data/rollup.config.js:7-17
Timestamp: 2025-01-24T11:46:49.990Z
Learning: When suggesting external dependencies in Rollup configuration, only include packages that are actually listed in the package's dependencies or peerDependencies.
Applied to files:
fdm-calculator/rollup.config.jsfdm-data/rollup.config.js
📚 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-calculator/rollup.config.js
📚 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-calculator/rollup.config.jsfdm-data/rollup.config.js
🧬 Code graph analysis (1)
fdm-data/rollup.config.js (1)
fdm-calculator/rollup.config.js (2)
isProd(6-6)external(8-11)
🔇 Additional comments (5)
fdm-calculator/src/doses/d.ts (2)
21-38: LGTM! Well-structured dose properties interface.The
DosePropertiesinterface is clearly documented and logically organized. Makingp_app_idoptional while keeping dose values as required numbers is appropriate for the domain.
44-44: NumericDoseKeys type definition is correct and properly integrated.The type correctly excludes
p_app_id(the only string-type property inDoseProperties) while retaining all 16 numeric dose properties (p_dose_n,p_dose_nw,p_dose_p, etc.). The usage incalculate-dose.ts(line 131) is type-safe:Object.keys(totalDose) as NumericDoseKeys[]properly constrains iteration to numeric properties only. No type safety issues found.fdm-data/rollup.config.js (1)
14-33: LGTM! Clean migration to esbuild with appropriate library bundling.The configuration successfully migrates from TypeScript + Terser to esbuild while:
- Using
preserveModulesfor library-friendly output structure- Dynamically computing externals from package.json (per established patterns)
- Optimizing sourcemaps by environment (inline for dev, separate for prod)
- Targeting Node 20 for modern JavaScript features
fdm-calculator/rollup.config.js (2)
13-28: LGTM! Consistent migration pattern with fdm-data.The base configuration mirrors the clean esbuild migration in fdm-data, maintaining consistency across the monorepo packages.
30-54: Placeholder replacement logic handles preserveModules correctly.The containment check and null return (line 35-37) appropriately handle the common case where most modules won't contain the version placeholder. The length validation and padding preserve sourcemap accuracy.
2 participants
Summary by CodeRabbit
Bug Fixes
Performance
Chores
✏️ Tip: You can customize this high-level summary in your review settings.