Conversation
Greptile SummaryThis PR upgrades the Console SDK to v9.0.0, migrating key and platform management from the
Confidence Score: 4/5Two P1 issues should be addressed before merging: identical platform enums that break type narrowing, and the scopes required/optional mismatch in createKey/updateKey. The platform type enums all contain identical values, making the discriminated-union split a no-op from a type-safety perspective. The scopes mismatch in createKey/updateKey will silently compile but throw at runtime. Both are straightforward fixes. src/enums/platform-*-type.ts (all five files) and src/services/project.ts (createKey/updateKey signatures)
|
| Filename | Overview |
|---|---|
| src/enums/platform-android-type.ts | New enum for Android platform type, but contains all 5 platform values instead of only 'android' |
| src/enums/platform-apple-type.ts | New enum for Apple platform type, but contains all 5 platform values instead of only 'apple' |
| src/enums/platform-web-type.ts | New enum for Web platform type, but contains all 5 platform values instead of only 'web' |
| src/enums/platform-windows-type.ts | New enum for Windows platform type, but contains all 5 platform values instead of only 'windows' |
| src/enums/platform-linux-type.ts | New enum for Linux platform type, but contains all 5 platform values instead of only 'linux' |
| src/services/project.ts | New Project service migrated from Projects; createKey/updateKey mark scopes optional in TS types but throw at runtime if omitted |
| src/services/projects.ts | Removed keys/platforms management methods; migrated to project-scoped endpoints in the new Project service |
| src/models.ts | Platform model refactored into 5 specific subtypes; PlatformList relocated and updated; AuditLog gains userType; BillingPlan gains email validation support flags |
| src/services/databases.ts | updateCollection gains purge parameter; listDocuments TTL docs improved |
| src/services/tables-db.ts | updateTable gains purge parameter; listRows TTL docs improved |
| src/index.ts | PlatformType removed; 5 new platform-specific type enums exported in its place |
| src/client.ts | SDK version bumped from 8.3.0 to 9.0.0 |
Reviews (1): Last reviewed commit: "chore: update Console SDK to 9.0.0" | Re-trigger Greptile
| export enum PlatformAndroidType { | ||
| Windows = 'windows', | ||
| Apple = 'apple', | ||
| Android = 'android', | ||
| Linux = 'linux', | ||
| Web = 'web', | ||
| } No newline at end of file |
There was a problem hiding this comment.
All platform-specific type enums are identical
All five new enums (PlatformAndroidType, PlatformAppleType, PlatformWebType, PlatformWindowsType, PlatformLinuxType) contain the exact same five values. This defeats the purpose of the discriminated-union design: PlatformAndroid.type is typed as PlatformAndroidType, but TypeScript will accept PlatformAndroidType.Windows for it — there is no compile-time narrowing to 'android' only.
Each enum should contain only its own platform value:
| export enum PlatformAndroidType { | |
| Windows = 'windows', | |
| Apple = 'apple', | |
| Android = 'android', | |
| Linux = 'linux', | |
| Web = 'web', | |
| } | |
| export enum PlatformAndroidType { | |
| Android = 'android', | |
| } |
The same single-value fix applies to the other four enums (PlatformAppleType = 'apple', PlatformWebType = 'web', PlatformWindowsType = 'windows', PlatformLinuxType = 'linux').
There was a problem hiding this comment.
Valid comment, something seems off here
There was a problem hiding this comment.
We shouldnt need those types anymore for create/update endpoinds. They are only relevant for get/list, so it should only exist once, probably inder PlatformType
There was a problem hiding this comment.
You're right — if these are only needed for get/list responses and not for discriminating between platform types in create/update, then a single PlatformType enum makes much more sense:
export enum PlatformType {
Windows = 'windows',
Apple = 'apple',
Android = 'android',
Linux = 'linux',
Web = 'web',
}That would eliminate the redundancy across all five files and still provide the type safety needed for get/list operations.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules and .greptile/config.
| * @throws {AppwriteException} | ||
| * @returns {Promise<Models.Key>} | ||
| */ | ||
| createKey(params: { keyId: string, name: string, scopes?: Scopes[], expire?: string }): Promise<Models.Key>; |
There was a problem hiding this comment.
scopes typed optional but throws if absent
The createKey overload marks scopes as optional (scopes?: Scopes[]), but the implementation throws AppwriteException('Missing required parameter: "scopes"') when it is undefined (line 291). TypeScript callers receive no compile-time error when they omit scopes, yet get a runtime exception.
| createKey(params: { keyId: string, name: string, scopes?: Scopes[], expire?: string }): Promise<Models.Key>; | |
| createKey(params: { keyId: string, name: string, scopes: Scopes[], expire?: string }): Promise<Models.Key>; |
| * @throws {AppwriteException} | ||
| * @returns {Promise<Models.Key>} | ||
| */ | ||
| updateKey(params: { keyId: string, name: string, scopes?: Scopes[], expire?: string }): Promise<Models.Key>; |
There was a problem hiding this comment.
scopes typed optional but throws if absent (updateKey)
Same mismatch as createKey — scopes is ? in the object-param overload and the positional overload, but the implementation throws at runtime when it is undefined (line 425).
| updateKey(params: { keyId: string, name: string, scopes?: Scopes[], expire?: string }): Promise<Models.Key>; | |
| updateKey(params: { keyId: string, name: string, scopes: Scopes[], expire?: string }): Promise<Models.Key>; |
| name: '<NAME>', | ||
| key: '<KEY>', // optional | ||
| store: '<STORE>', // optional | ||
| hostname: '' // optional |
There was a problem hiding this comment.
Can we find some way to mark hostname required? 🤔
| const result = await project.updateWebPlatform({ | ||
| platformId: '<PLATFORM_ID>', | ||
| name: '<NAME>', | ||
| hostname: '' // optional |
| @@ -0,0 +1,7 @@ | |||
| export enum PlatformAppleType { | |||
There was a problem hiding this comment.
Notice we have 5 such type enums
| @@ -1,17 +0,0 @@ | |||
| export enum PlatformType { | |||
There was a problem hiding this comment.
This should probably still exist, but with only 5 platforms.
This PR contains updates to the Console SDK for version 9.0.0.