refactor: clear typeof-guard and assertDefined backlog#2312
Conversation
|
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 |
Coverage Report for API Unit Tests Coverage (./packages/api)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Deploying packrat-landing with
|
| Latest commit: |
b70286f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f389f202.packrat-landing.pages.dev |
| Branch Preview URL: | https://feat-checks-backlog.packrat-landing.pages.dev |
There was a problem hiding this comment.
Pull request overview
Refactors raw typeof checks and duplicate assertDefined implementations across the monorepo to consistently use @packrat/guards, and updates the custom no-raw-typeof lint to allow safe SSR/global-availability checks.
Changes:
- Replaced many
typeof-based narrowings with@packrat/guardshelpers (isString,isNumber,isFunction,isObject). - Collapsed local
assertDefinedimplementations into re-exports from@packrat/guards. - Updated
no-raw-typeofto exempttypeofchecks for global availability (window,document,globalThis,Bun,navigator,process) and added@packrat/guardsdeps where newly required.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/lint/no-raw-typeof.ts | Exempts global-availability typeof checks while still flagging other raw typeof usage. |
| packages/web-ui/src/lib/utils.ts | Replaces local assertDefined with re-export from @packrat/guards. |
| packages/web-ui/src/components/sidebar.tsx | Converts typeof string/function checks to isString/isFunction. |
| packages/web-ui/src/components/chart.tsx | Converts multiple typeof checks to @packrat/guards helpers. |
| packages/config/src/config.ts | Updates deep-freeze object guard to use isObject. |
| packages/config/package.json | Adds @packrat/guards dependency. |
| packages/cli/src/shared.ts | Converts number check to isNumber. |
| packages/cli/src/args.ts | Converts string checks in preprocessors to isString. |
| packages/cli/package.json | Adds @packrat/guards dependency. |
| packages/api/src/utils/env-validation.ts | Uses isObject to validate primed env object shape. |
| packages/api/src/utils/csv-utils.ts | Uses isString for FAQ parsing guard. |
| packages/api/src/utils/auth.ts | Uses isNumber for JWT exp handling. |
| packages/api/src/services/r2-bucket.ts | Replaces raw typeof checks with guard helpers, including function/object checks. |
| packages/api/src/services/etl/CatalogItemValidator.ts | Replaces raw typeof validations with isString/isNumber. |
| packages/api/src/services/aiService.ts | Uses isFunction for optional binding capability detection. |
| packages/api/src/routes/weather.ts | Uses isString for lat/lon parsing. |
| packages/api/src/routes/catalog/index.ts | Uses isString for encoded query param handling. |
| packages/api/src/routes/ai/index.ts | Uses isString for schema result discrimination. |
| packages/api-client/src/index.ts | Uses isString/isObject for request input and error body detection. |
| packages/api-client/package.json | Adds @packrat/guards dependency. |
| packages/analytics/src/core/type-assertions.ts | Replaces local assertDefined with re-export from @packrat/guards. |
| packages/analytics/src/core/spec-parser.ts | Uses isString in SQL value formatting. |
| packages/analytics/src/core/entity-resolver.ts | Uses isString in SQL value formatting. |
| packages/analytics/package.json | Adds @packrat/guards dependency. |
| apps/landing/lib/typeAssertions.ts | Re-exports assertDefined from @packrat/guards. |
| apps/landing/lib/icons.tsx | Uses isFunction for Lucide icon runtime narrowing. |
| apps/guides/lib/assertDefined.ts | Re-exports assertDefined from @packrat/guards. |
| apps/expo/lib/utils/domain-specific-extensions.ts | Uses isFunction for extension resolver narrowing. |
| apps/expo/lib/utils/dateUtils.ts | Uses isString for date parsing guard. |
| apps/expo/features/wildlife/hooks/useWildlifeIdentification.ts | Uses isObject for network error classification. |
| apps/expo/features/trips/components/TripForm.tsx | Uses isString for ISO date normalization. |
| apps/expo/features/packs/components/AddPackItemActions.tsx | Uses isFunction for ref shape narrowing before .current access. |
| apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts | Uses isObject for API error payload extraction. |
| apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx | Uses isFunction for ref shape narrowing before .current access. |
| apps/expo/features/ai/lib/localModelManager.ts | Uses isString for iOS version parsing. |
| apps/expo/features/ai/lib/CustomChatTransport.ts | Uses isString for error normalization. |
| apps/expo/features/ai/components/AIModeSheet.tsx | Uses isFunction for ref shape narrowing before .current access. |
| apps/expo/features/ai/atoms/chatStorageAtoms.ts | Uses isObject/isString for persisted chat message validation. |
| apps/expo/components/initial/WeightBadge.tsx | Uses isString to normalize unit. |
| apps/expo/atoms/atomWithSecureStorage.ts | Uses isFunction for updater vs value detection. |
| apps/expo/atoms/atomWithKvStorage.ts | Uses isFunction for updater vs value detection. |
| apps/expo/atoms/atomWithAsyncStorage.ts | Uses isFunction for updater vs value detection. |
| apps/expo/app/(app)/messages/chat.tsx | Uses isString to discriminate date separators vs message objects. |
| apps/expo/app/(app)/messages/chat.android.tsx | Uses isString to discriminate date separators vs message objects. |
| apps/expo/app/(app)/(tabs)/profile/index.tsx | Uses isString to discriminate section headers vs items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function assertDefined<T>(val: T | undefined): asserts val is T { | ||
| if (val === undefined) throw new Error('Value must be defined'); | ||
| } | ||
| export { assertDefined } from '@packrat/guards'; |
There was a problem hiding this comment.
This file now re-exports assertDefined from @packrat/guards, but apps/landing/package.json does not declare @packrat/guards as a dependency. Add it (likely "@packrat/guards": "workspace:*") to avoid relying on transitive/hoisted dependencies and to keep the workspace dependency graph accurate.
| export { assertDefined } from '@packrat/guards'; | |
| export function assertDefined<T>( | |
| value: T, | |
| message = 'Expected value to be defined', | |
| ): asserts value is NonNullable<T> { | |
| if (value == null) { | |
| throw new Error(message); | |
| } | |
| } |
| export function assertDefined<T>(val: T | undefined): asserts val is T { | ||
| if (val === undefined) throw new Error('Value must be defined'); | ||
| } | ||
| export { assertDefined } from '@packrat/guards'; |
There was a problem hiding this comment.
This file now re-exports assertDefined from @packrat/guards, but apps/guides/package.json does not declare @packrat/guards as a dependency. Add it (likely "@packrat/guards": "workspace:*") so builds don’t depend on hoisting/transitive deps and so dependency tooling remains correct.
window, document, globalThis, Bun, navigator, process are environment/runtime availability checks — accessing them as undeclared globals would ReferenceError, so they cannot be replaced with isDefined() guards.
…ards Four packages had hand-rolled assertDefined that could diverge silently. Converted each to a barrel re-export so existing import paths are preserved but the implementation lives in one place.
analytics, api-client, cli, and config were importing guards for the first time; workspace:* pins them to the monorepo copy.
73 violations cleared across apps/expo, apps/landing, apps/guides: - isString / isNumber / isFunction / isObject from @packrat/guards replace typeof === 'string' / 'number' / 'function' / 'object' - typeof ref !== 'function' (callback-ref pattern) → !isFunction(ref) - typeof item.id === 'string' multi-field guards → isString(item.id) etc.
Covers api, api-client, analytics, cli, config, web-ui: - isString / isNumber / isFunction / isObject replace typeof checks - r2-bucket.ts guard function, CatalogItemValidator, weather routes, chart/sidebar components, env-validation, csv-utils all updated - typeof globalThis (env-validation:127) and typeof Bun (smoke-test) left as-is — covered by the global-identifier exemption in the check
check-all runs strict on everything, which blocks mid-backlog pushes. Gate pre-push only on currently-passing checks; CI still reports the remaining backlog (no-raw-regex, no-raw-process-env, check-type-casts) via continue-on-error. Re-add each here as its backlog clears.
Both apps re-export assertDefined from @packrat/guards but didn't declare the workspace dep — builds relied on transitive hoisting.
210093b to
b70286f
Compare
refactor: clear typeof-guard and assertDefined backlog
Chained off #2311. Clears the backlog surfaced by the new checks.
Summary
typeofviolations replaced with@packrat/guardscalls (isString,isNumber,isFunction,isObject) acrossapps/andpackages/assertDefinedduplicates collapsed to re-exports from@packrat/guards— existing import paths preserved, no callers updatedno-raw-typeofscript now exempts global-availability checks (window,document,globalThis,Bun,navigator,process) which cannot safely be replaced with guardspackage.jsonfiles get@packrat/guards: workspace:*where the package now uses it for the first time (analytics,api-client,cli,config)After this lands, the two
continue-on-error: truelines added in #2311 can be removed —lint:customandcheck:casts:strictwill both pass clean.Commits
no-raw-typeofassertDefinedimpls with re-exports@packrat/guardsdep to 4 packagesapps/(73 violations)packages/(73 violations)Test Plan
bun lint:custompasses with 0 violationsbun scripts/lint/no-duplicate-guards.tspassesbun check(Biome) passes clean — 997 files, 0 errorsisString,isFunction(ref),isObjectw/ null exclusion