chore: migrate scattered type guards to @packrat/guards#2039
Merged
Conversation
Introduce a shared guards package so every app in the monorepo has one canonical import path for runtime type narrowing instead of reaching into radash directly, copying assertion helpers into per-app typeAssertions.ts, or leaning on `as` casts at API → store boundaries. ## What's in it - **radash re-exports**: isString, isNumber, isDate, isObject, isArray, isBoolean, isEmpty, isEqual, isFloat, isFunction, isInt, isPrimitive, isPromise, isSymbol. - **assertions** (throw on failure, narrow via `asserts`): assertDefined, assertNonNull, assertPresent, assertIsString, assertIsNumber, assertIsBoolean, assertAllDefined. - **narrow** (return T | undefined instead of throwing): asString, asNumber, asBoolean, asDate, asStringRecord, nullToUndefined. - **enum** (string literal union validation): makeEnumGuard, assertEnum. ## Wiring - New workspace package at packages/guards (private, bun workspaces picks it up via packages/*). - tsconfig paths entries for @packrat/guards and @packrat/guards/*. - radash as a direct dependency. ## Not yet A follow-up pass will migrate existing scattered `as` casts and per-app typeAssertions.ts copies over to @packrat/guards. Keeping this commit scoped to the new package so the blast radius is zero.
Delete duplicated assertion helpers in apps/expo and packages/api and re-point all consumers at the canonical implementations in @packrat/guards. No behavioural changes — symbol names are preserved.
Switch the two @packrat/api call sites that import isString/isArray from radash to pull them from @packrat/guards instead, keeping all runtime narrowing on the canonical import path.
Use the narrowing helpers from @packrat/guards to convert the unknown gray-matter frontmatter values in getGuidesRoute instead of blind casts to string/number.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
dependencies
Contributor
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Contributor
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates runtime type narrowing/assertions across apps/expo and packages/api to the new workspace package @packrat/guards, removing duplicated per-app typeAssertions helpers and aligning API code to use the shared guard/narrowing utilities.
Changes:
- Added the
@packrat/guardsworkspace package (assertions, narrowing helpers, enum helpers; radash guard re-exports) and wired TS path aliases. - Updated Expo + API call sites to import assertions/guards from
@packrat/guards, deleting the old duplicatedtypeAssertions.tsfiles (and their tests). - Replaced some unsafe
as string/as numbercasts in the Guides route withasString/asNumbernarrowers and migrated select radash imports to@packrat/guards.
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds path aliases for @packrat/guards (and subpaths). |
| packages/guards/package.json | Defines the new @packrat/guards workspace package and its exports/deps. |
| packages/guards/src/index.ts | Central export surface (radash guard re-exports + internal helpers). |
| packages/guards/src/assertions.ts | New assertion helpers (assertDefined, assertNonNull, etc.). |
| packages/guards/src/narrow.ts | New non-throwing narrowers (asString, asNumber, etc.). |
| packages/guards/src/enum.ts | New enum guard/assert helpers (makeEnumGuard, assertEnum). |
| packages/api/package.json | Adds @packrat/guards workspace dependency. |
| packages/api/test/utils/db-helpers.ts | Switches assertion import to @packrat/guards. |
| packages/api/src/utils/typeAssertions.ts | Deletes API-local assertion helpers. |
| packages/api/src/utils/tests/typeAssertions.test.ts | Deletes API-local assertion helper tests. |
| packages/api/src/services/r2-bucket.ts | Repoints isString import from radash to @packrat/guards. |
| packages/api/src/routes/packTemplates/packTemplates.ts | Repoints assertDefined import to @packrat/guards. |
| packages/api/src/routes/guides/getGuidesRoute.ts | Uses asString/asNumber and imports isArray from @packrat/guards. |
| packages/api/src/routes/auth/index.ts | Repoints assertDefined import to @packrat/guards. |
| packages/api/src/routes/admin/index.ts | Repoints assertAllDefined import and updates call to array signature. |
| apps/expo/package.json | Adds @packrat/guards workspace dependency. |
| apps/expo/utils/typeAssertions.ts | Deletes Expo-local assertion helpers. |
| apps/expo/utils/tests/typeAssertions.test.ts | Deletes Expo-local assertion helper tests. |
| apps/expo/features/weather/lib/weatherService.ts | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/weather/components/LocationPicker.tsx | Repoints assertNonNull import to @packrat/guards. |
| apps/expo/features/trips/screens/TripDetailScreen.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/trips/screens/EditTripScreen.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/trips/components/TripForm.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/utils/getPackItemDetailOptions.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/utils/computeCategories.ts | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/screens/ItemsScanScreen.tsx | Repoints assertNonNull import to @packrat/guards. |
| apps/expo/features/packs/screens/EditPackScreen.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/hooks/usePackWeightHistory.ts | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/hooks/useImagePicker.ts | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/components/PackItemCard.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/packs/components/GapItemCatalogSuggestions.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/pack-templates/utils/getPackTemplateItemDetailOptions.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/pack-templates/screens/PackTemplateItemDetailScreen.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/features/pack-templates/screens/ItemsScanScreen.tsx | Repoints assertNonNull import to @packrat/guards. |
| apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/app/(app)/season-suggestions.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/app/(app)/messages/conversations.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/app/(app)/messages/conversations.android.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/app/(app)/messages/chat.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/app/(app)/gear-inventory.tsx | Repoints assertDefined import to @packrat/guards. |
| apps/expo/app/(app)/(tabs)/(home)/index.tsx | Repoints assertIsString import to @packrat/guards. |
| bun.lock | Updates lockfile to include the new workspace and other dependency resolution changes. |
Comment on lines
+10
to
+14
| export { | ||
| isArray, | ||
| isDate, | ||
| isEmpty, | ||
| isEqual, |
There was a problem hiding this comment.
The public re-export list doesn’t appear to include all of the primitive guards this package claims to centralize (e.g. isBoolean isn’t exported here). Either add the missing radash guard re-exports, or tighten the header comment so it doesn’t imply full coverage.
Comment on lines
+6
to
+10
| "main": "./src/index.ts", | ||
| "types": "./src/index.ts", | ||
| "exports": { | ||
| ".": "./src/index.ts" | ||
| }, |
There was a problem hiding this comment.
tsconfig.json defines a @packrat/guards/* path alias, but this package’s exports map only exposes .. If someone imports a subpath (e.g. @packrat/guards/narrow), TypeScript may resolve it but runtime/bundlers will fail. Consider either adding an ./* export entry (or explicit subpath exports) or removing the @packrat/guards/* path mapping.
Comment on lines
+40
to
+45
| /** | ||
| * Returns a `Record<string, string>` from an unknown value, keeping only | ||
| * string-valued entries. Returns `{}` if the input isn't a plain object. | ||
| */ | ||
| export const asStringRecord = (value: unknown): Record<string, string> => { | ||
| if (value === null || typeof value !== 'object') return {}; |
There was a problem hiding this comment.
asStringRecord’s docstring says it returns {} when the input isn’t a plain object, but the implementation treats arrays as objects and will iterate their entries. Either explicitly exclude arrays (and other non-plain objects) or update the comment to match the actual behavior.
Comment on lines
+9
to
+14
| export function assertDefined<T>( | ||
| value: T | undefined, | ||
| message = 'Value must be defined', | ||
| ): asserts value is T { | ||
| if (value === undefined) throw new Error(message); | ||
| } |
There was a problem hiding this comment.
This PR deletes the per-app typeAssertions unit tests, but there are no equivalent tests added for @packrat/guards assertions. To avoid a coverage regression, consider moving/rewriting the old tests under packages/guards (covering assertDefined, assertAllDefined, etc.).
Comment on lines
98
to
+100
| category: obj.customMetadata?.category || 'general', | ||
| categories: (frontmatter.categories as string[]) || [], | ||
| description: (frontmatter.description as string) || obj.customMetadata?.description || '', | ||
| author: frontmatter.author as string, | ||
| readingTime: frontmatter.readingTime as number, | ||
| difficulty: frontmatter.difficulty as string, | ||
| description: asString(frontmatter.description) || obj.customMetadata?.description || '', |
There was a problem hiding this comment.
frontmatter.categories as string[] is an unchecked cast from unknown and can produce a non-array value in the response (violating GuideSchema.categories). Since you already have isArray/isString available from @packrat/guards, prefer runtime narrowing (and filtering to strings) before returning categories.
andrew-bierman
added a commit
that referenced
this pull request
Apr 11, 2026
Finishes the migration started in #2039: - apps/expo/lib/utils/itemCalculations.ts: replace the inline isWeightUnit guard with makeEnumGuard from @packrat/guards and export it so other call sites can reuse it - apps/expo/features/packs/components/PackCard.tsx: import isArray from @packrat/guards instead of radash directly - apps/expo/features/pack-templates/components/PackTemplateCard.tsx: same - apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts: use the canonical isWeightUnit from itemCalculations instead of re-declaring an inline copy
3 tasks
andrew-bierman
added a commit
that referenced
this pull request
May 14, 2026
chore: migrate scattered type guards to @packrat/guards
andrew-bierman
added a commit
that referenced
this pull request
May 14, 2026
Finishes the migration started in #2039: - apps/expo/lib/utils/itemCalculations.ts: replace the inline isWeightUnit guard with makeEnumGuard from @packrat/guards and export it so other call sites can reuse it - apps/expo/features/packs/components/PackCard.tsx: import isArray from @packrat/guards instead of radash directly - apps/expo/features/pack-templates/components/PackTemplateCard.tsx: same - apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts: use the canonical isWeightUnit from itemCalculations instead of re-declaring an inline copy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates all scattered type narrowing in apps/expo and packages/api onto the new
@packrat/guardspackage introduced in #2038. This PR chains ontofeat/guards-package, so #2038 must land first.What changed
1.
typeAssertions.tscopies removedDeleted the duplicated per-app assertion helpers and re-pointed all consumers at
@packrat/guards:apps/expo/utils/typeAssertions.ts(and its test)packages/api/src/utils/typeAssertions.ts(and its test)All 22 import sites (20 in expo, 4 in api/test) now use
import { assertDefined, assertNonNull, assertIsString, assertAllDefined } from '@packrat/guards'. TheassertAllDefinedcall inpackages/api/src/routes/admin/index.tswas updated to pass an array, matching the new signature in@packrat/guards(the deleted api copy used a variadic signature; the new canonical one takesreadonly unknown[]).Both
apps/expoandpackages/apinow declare@packrat/guardsas a workspace dependency.2. radash guard re-exports
Switched the two
@packrat/apicall sites that imported guards fromradashdirectly to import from@packrat/guards:packages/api/src/services/r2-bucket.ts—isStringpackages/api/src/routes/guides/getGuidesRoute.ts—isArrayExpo files that still import
isArrayfromradash(PackCard.tsx,PackTemplateCard.tsx) were left alone per the scope rules.3. Cast migration
In
packages/api/src/routes/guides/getGuidesRoute.ts, replaced blindas string/as numbercasts ongray-matterfrontmatter (Record<string, unknown>) withasString/asNumberfrom@packrat/guards. Thefrontmatter.categories as string[]cast was left in place because there's no array-of-strings helper yet and adding one is out of scope.What I deliberately did NOT migrate
useGenerateTemplateFromOnlineContent.ts: the task description mentioned this file contained an inlineisWeightUnitguard and aWEIGHT_UNITSset to replace withmakeEnumGuard, but on inspection the file doesn't contain either of those. The actualisWeightUnitguard lives inapps/expo/lib/utils/itemCalculations.ts, which is outside the allowed scope (expo touches limited to step 1). Skipped to stay in-scope.isString/isNumber/isDate/isObjectin r2-bucket.ts: the task mentioned these, but the file only actually usesisString, which is already migrated in step 2.as string[]ingetCategoriesRoute.tsandgetGuidesRoute.ts: no array-narrowing helper in@packrat/guards; adding one is out of scope.{} as Record<string, SQL>incatalogService.ts— these aren't narrowing fromunknown.Verification
bun run check-types— clean (no new errors)bun lint— 185 warnings, all pre-existing (same count as baseline before this PR); no errorsPrerequisite
Merges onto
feat/guards-package(#2038). Re-target todevelopmentonly after #2038 is merged.Test plan
@packrat/guardsis available on the base branchcheck-typespasseslintpasses/api/guides) still behaves correctly after the cast → narrow change