fix: resolve TypeScript build errors#793
Conversation
- Add missing isShortcutKey type guard to keybinding.ts
- Add missing isActionWithOptionalArgs and isAction type guards to types.ts
- Fix IndexedDBAdapter constructor calls to use object params instead of positional args (runner.ts, v1-to-v2.ts)
- Fix StickersRegistry.register call to use object param { key, definition }
|
@hevink is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds runtime type guards: ChangesAction Validation & Keybinding Type Guards
API Refactoring to Object-Based Parameters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/actions/types.ts (1)
34-42: ⚡ Quick winMove the constant Set to module level for better performance.
The
ACTIONS_WITH_REQUIRED_ARGSSet is recreated on every function call. Following the same pattern asMODIFIER_KEYS_SETinkeybinding.tsandACTION_KEYS_SETin this file, declare it as a module-level constant.♻️ Proposed refactor
const ACTION_KEYS_SET: ReadonlySet<string> = new Set(Object.keys(ACTIONS)); +const ACTIONS_WITH_REQUIRED_ARGS: ReadonlySet<string> = new Set([ + "remove-media-asset", + "remove-media-assets", +]); + export function isAction(value: string): value is TAction { return ACTION_KEYS_SET.has(value); } export function isActionWithOptionalArgs(value: string): value is TActionWithOptionalArgs { if (!isAction(value)) return false; // Actions that require mandatory (non-undefined) args cannot be used as keybindings - const ACTIONS_WITH_REQUIRED_ARGS: ReadonlySet<string> = new Set([ - "remove-media-asset", - "remove-media-assets", - ]); return !ACTIONS_WITH_REQUIRED_ARGS.has(value); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/actions/types.ts` around lines 34 - 42, The ACTIONS_WITH_REQUIRED_ARGS Set is recreated on every call inside isActionWithOptionalArgs; pull that Set out to module scope as a top-level constant (e.g., export or const ACTIONS_WITH_REQUIRED_ARGS = new Set([...])) so it is instantiated once, mirroring the pattern used for MODIFIER_KEYS_SET and ACTION_KEYS_SET (see keybinding.ts and this file), then update isActionWithOptionalArgs to reference the module-level ACTIONS_WITH_REQUIRED_ARGS.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/actions/types.ts`:
- Around line 34-42: The ACTIONS_WITH_REQUIRED_ARGS Set is recreated on every
call inside isActionWithOptionalArgs; pull that Set out to module scope as a
top-level constant (e.g., export or const ACTIONS_WITH_REQUIRED_ARGS = new
Set([...])) so it is instantiated once, mirroring the pattern used for
MODIFIER_KEYS_SET and ACTION_KEYS_SET (see keybinding.ts and this file), then
update isActionWithOptionalArgs to reference the module-level
ACTIONS_WITH_REQUIRED_ARGS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc1c898f-c2f9-45e0-9a67-1730bd79be04
📒 Files selected for processing (5)
apps/web/src/actions/keybinding.tsapps/web/src/actions/types.tsapps/web/src/services/storage/migrations/runner.tsapps/web/src/services/storage/migrations/v1-to-v2.tsapps/web/src/stickers/providers/index.ts
Per CodeRabbit review: avoid recreating the Set on every function call by declaring it as a module-level constant, consistent with the pattern used for MODIFIER_KEYS_SET and ACTION_KEYS_SET.
Summary
Fixes all 5 TypeScript errors reported in #782 that cause
bun run buildand Docker builds to fail onmain.Closes #782
Changes
1.
src/actions/keybinding.tsAdded missing
isShortcutKeytype guard function thatpersistence.tsimports but didn't exist.2.
src/actions/types.tsAdded missing
isActionandisActionWithOptionalArgstype guard functions thatpersistence.tsimports but didn't exist.ACTIONS_WITH_REQUIRED_ARGSis declared at module scope to avoid recreating the Set on every call.3.
src/services/storage/migrations/runner.ts&v1-to-v2.tsFixed
IndexedDBAdapterconstructor calls — the constructor takes a single object{ dbName, storeName, version }but was being called with 3 positional arguments (3 occurrences).Also fixed
projectsAdapter.set(projectId, result.project)→projectsAdapter.set({ key: projectId, value: result.project })to match the adapter's actual API.4.
src/stickers/providers/index.tsFixed
stickersRegistry.register(id, provider)→stickersRegistry.register({ key: id, definition: provider })to matchDefinitionRegistry.register's object param signature.Testing
bun run buildnow completes successfully ✅