Skip to content

refactor(ui-components): remove ApiKeysCard and ApiKeyTable - #52

Merged
jfrench9 merged 1 commit into
mainfrom
refactor/remove-api-keys-card
Aug 28, 2026
Merged

jfrench9 merged 1 commit into
mainfrom
refactor/remove-api-keys-card

Conversation

@jfrench9

Copy link
Copy Markdown
Member

Summary

Removes ApiKeysCard and ApiKeyTable from ui-components/api-keys/. They were only ever rendered by robosystems-app's /settings, and robosystems-app#343 replaces that usage with an app-local card (settings is the login home, so its cards live in the app). Nothing else — in this package or in any consuming app — imports them. CreateApiKeyModal, ApiKeyDisplay, SecureApiKeyField, and the ApiKey / ApiKeyWithValue types stay: the app-local card still creates keys through the core modal.

Changes

  • ui-components/api-keys/ — deleted ApiKeysCard.tsx, ApiKeyTable.tsx, and __tests__/ApiKeyTable.test.tsx; the barrel now exports ApiKeyDisplay, CreateApiKeyModal, SecureApiKeyField only.
  • index.ts — dropped ApiKeysCard and ApiKeyTable from the root barrel (ui-components/index.ts re-exports the directory barrel, so the @robosystems/core/ui-components subpath loses them too).
  • No changes to ui-components/typesApiKey / ApiKeyWithValue remain the CreateApiKeyModal contract.

Consumer Impact

BREAKING — two removed exports: ApiKeysCard and ApiKeyTable, from both the root barrel and @robosystems/core/ui-components. Cut the release as a minor (0.9.0), not a patch.

  • robosystems-app: the only consumer. Its /settings stops importing ApiKeysCard in robosystems-app#343merge #343 before bumping core there; bumping first would break its build.
  • roboledger-app, roboinvestor-app: nothing to change — neither ever imported the removed exports (grep clean, as are harbinger-finlab-app and the integration template).
  • Packaging: unchanged — no directive, ESM, publish-from-dist, sideEffects, or peer-range changes; the build is purely subtractive.

Testing

  • npm run test:all — green: format:check, lint (0 errors; the 12 warnings are the pre-existing no-unused-vars debt in untouched files), typecheck, vitest 43 files / 492 tests (16 skipped), build.
  • Validated in a real app: npm run pack:local, then in robosystems-app on the #343 branch npm install <tgz> --no-savenpm run typecheck clean, /settings card tests 15/15, and the installed package confirmed free of the removed exports. node_modules re-synced from the lockfile afterwards (app tree left clean).
  • Not validated in roboledger-app / roboinvestor-app — they never imported the removed exports.

Only robosystems-app's /settings ever rendered them, and
robosystems-app#343 replaces that with an app-local card — settings is
the login home, so its cards live in the app. No other consumer imports
either export. CreateApiKeyModal, ApiKeyDisplay, SecureApiKeyField, and
the ApiKey / ApiKeyWithValue types stay; the app-local card still
creates keys through the core modal.

Breaking: two removed exports from the root barrel and
@robosystems/core/ui-components — cut as a minor. Merge
robosystems-app#343 before bumping core there.
@jfrench9

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @jfrench9's task in 49s —— View job


Reviewing PR #52

  • Gather context (PR diff, changed files)
  • Verify no remaining references to ApiKeysCard / ApiKeyTable
  • Review barrel export changes
  • Check for stray references (docs, types, tests)
  • Post review feedback

Review

Clean, purely-subtractive removal — nothing to flag.

  • No dangling references: grep -rn "ApiKeysCard|ApiKeyTable" across the repo returns zero matches outside the deleted files. The ui-components/api-keys/index.ts barrel (ui-components/api-keys/index.ts:1-3) and root index.ts (index.ts:179-186) both drop the two exports cleanly, and no other barrel re-exports them.
  • No orphaned dependencies: ApiKeysCard.tsx was the only consumer that pulled in SettingsCard and GraphContext from this subtree — both are still used elsewhere (ui-components/settings/*, contexts/*, several components/*), so nothing else needs deleting.
  • Types correctly retained: ApiKey / ApiKeyWithValue in ui-components/types are untouched, matching the PR description — they're still the CreateApiKeyModal contract, which stays exported.
  • Test removal is consistent: __tests__/ApiKeyTable.test.tsx is deleted alongside the component it tested; no other test imports ApiKeyTable or ApiKeysCard.
  • Versioning: description correctly calls for a minor bump (0.9.0) given the two breaking export removals — consistent with this being an unscoped/public package where any export removal is breaking.
  • Sequencing risk called out appropriately: the PR body flags that robosystems-app#343 must merge before this gets consumed there, which is the right order to avoid breaking that app's build.

One minor nit (non-blocking): ApiKeysCard.tsx (now deleted) contained the only fetch/create/revoke logic against the SDK (listUserApiKeys, createUserApiKey, revokeUserApiKey) for this feature — worth double-checking the app-local replacement in robosystems-app#343 reimplements equivalent error handling (e.g., the isSystem guard against revoking system keys) since that behavior doesn't carry over automatically.

I wasn't able to run npm run typecheck / npm run test:all in this sandbox (Bash tool calls for npm run ... required approval that wasn't available here) — relying on static diff review and repo-wide grep instead. The PR description states test:all was already run green locally, which I have no reason to doubt given the diff is purely deletions.

Verdict: LGTM.

@jfrench9
jfrench9 merged commit a8368ac into main Aug 28, 2026
5 checks passed
@jfrench9
jfrench9 deleted the refactor/remove-api-keys-card branch August 28, 2026 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant