Repository navigation
feat: Node.js SDK update for version 30.0.1 - #177
Conversation
ChiragAgg5k
changed the title
ChiragAgg5k
changed the title
🟡 Tier B · Needs changes before merging
Updates the Node.js SDK to 30.0.1 and targets Appwrite response format 2.5.0. It adds query-building and topic helpers, updates service APIs and response models, and adds examples and tests for the expanded SDK surface. 56 of 143 changed files were too large to include in full.
Note Not approving while a bug finding is open: Accept query builders in service query parameter types
Fix with agent prompt### Issue 1
src/client.ts:99-102
**Add coverage for default filling and per-model copies**
The new defaults path changes responses with omitted fields and is intended to give each model independent nested defaults, but the existing and added tests do not exercise either case. A focused test would catch regressions that can leave fields absent or let a mutation on one returned model affect another.
### Issue 2
src/query.ts:976
**Accept query builders in service query parameter types**
The new request normalization handles `Query.Builder` values, but service methods such as `TablesDB.listRows` still declare `queries` as `string[]`. TypeScript callers therefore cannot pass the builder values supported by the new runtime path without a cast; update the query-bearing service parameter types and overloads to accept `QueryInput`.
### Issue 3
src/index.ts:44
**Document the removed wildcard Topic API**
This release removes `Topic.any()`/`.all()` and the `ResolvedTopic` export that were available in 30.0.0-rc.2, so existing wildcard-topic code now fails to compile or throws at runtime. Since the v30 breaking-changes list does not mention this removal, please retain the API or document this incompatibility.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 6
⏳ Still open from earlier reviews · 2
✅ Fixed since the last review · 1
Reviewed |
![hansi-codes[bot]](https://avatars.githubusercontent.com/in/5046327?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔵 Tier A · See the inline comments. Summary
src/client.ts:82-82
Clone mutable defaults for each returned model
Spreading model.defaults is shallow, so repeated response items missing an array field receive the same array instance. Mutating that field on one returned item then changes the other items too; clone mutable default values per model.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/client.ts
Line: 82
Comment:
**Clone mutable defaults for each returned model**
Spreading `model.defaults` is shallow, so repeated response items missing an array field receive the same array instance. Mutating that field on one returned item then changes the other items too; clone mutable default values per model.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
src/services/backups.ts:1005-1005
Keep restoration options object-shaped
BackupRestoration.options is typed and documented as an object, but this missing-field fallback supplies an array. When a response omits options, callers receive the wrong shape; the same fallback is repeated for restoration list and get responses.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/backups.ts
Line: 1005
Comment:
**Keep restoration options object-shaped**
`BackupRestoration.options` is typed and documented as an object, but this missing-field fallback supplies an array. When a response omits `options`, callers receive the wrong shape; the same fallback is repeated for restoration list and get responses.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
src/services/waf.ts:294-294
Keep WAF rule config object-shaped
WAF rule models define config as an object, but this fallback supplies an array. A response missing config therefore violates the returned model contract; the same default is used by the other WAF rule response handlers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/waf.ts
Line: 294
Comment:
**Keep WAF rule config object-shaped**
WAF rule models define `config` as an object, but this fallback supplies an array. A response missing `config` therefore violates the returned model contract; the same default is used by the other WAF rule response handlers.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
src/services/project.ts:77-77
Keep project onboarding object-shaped
The Project model defines onboarding as an object, but this fallback supplies an array. When a project response omits the field, callers get the wrong shape; the same fallback appears in the other project response defaults.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/project.ts
Line: 77
Comment:
**Keep project onboarding object-shaped**
The `Project` model defines `onboarding` as an object, but this fallback supplies an array. When a project response omits the field, callers get the wrong shape; the same fallback appears in the other project response defaults.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
src/services/advisor.ts:146-146
Keep CTA params object-shaped
InsightCTA.params is an object parameter map, but its missing-field fallback is an array. A CTA returned without params will consequently expose the wrong shape; this default is repeated for the other insight response methods.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/advisor.ts
Line: 146
Comment:
**Keep CTA params object-shaped**
`InsightCTA.params` is an object parameter map, but its missing-field fallback is an array. A CTA returned without `params` will consequently expose the wrong shape; this default is repeated for the other insight response methods.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
src/services/messaging.ts:119-119
Keep message data object-shaped
The Message model defines data as an object, but this fallback supplies an array. Responses omitting the field will return the wrong shape to consumers expecting keyed message data; the same default is repeated across message response methods.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/messaging.ts
Line: 119
Comment:
**Keep message data object-shaped**
The `Message` model defines `data` as an object, but this fallback supplies an array. Responses omitting the field will return the wrong shape to consumers expecting keyed message data; the same default is repeated across message response methods.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
src/services/messaging.ts:2376-2376
Keep provider credentials object-shaped
Provider models define credentials as an object, but this fallback supplies an array. When a provider response omits credentials, callers receive the wrong shape; this default is repeated across provider response methods.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/messaging.ts
Line: 2376
Comment:
**Keep provider credentials object-shaped**
Provider models define `credentials` as an object, but this fallback supplies an array. When a provider response omits `credentials`, callers receive the wrong shape; this default is repeated across provider response methods.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
| } | ||
| } | ||
|
|
||
| export type QueryInput = string | Query | Query.Builder | QueryInput[]; |
There was a problem hiding this comment.
Accept query builders in service query parameter types
The new request normalization handles Query.Builder values, but service methods such as TablesDB.listRows still declare queries as string[]. TypeScript callers therefore cannot pass the builder values supported by the new runtime path without a cast; update the query-bearing service parameter types and overloads to accept QueryInput.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/query.ts
Line: 976
Comment:
**Accept query builders in service query parameter types**
The new request normalization handles `Query.Builder` values, but service methods such as `TablesDB.listRows` still declare `queries` as `string[]`. TypeScript callers therefore cannot pass the builder values supported by the new runtime path without a cast; update the query-bearing service parameter types and overloads to accept `QueryInput`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
There was a problem hiding this comment.
Intentional for this release: service queries params stay string[] and the documented path is Query.builder()...build(). Widening every query-bearing param to accept builders needs the spec to mark those params, which it does not today, so it is a follow-up rather than part of this major.
There was a problem hiding this comment.
build() is a documented workaround, but the type/runtime mismatch remains: flattenParam recognizes and normalizes Query.Builder values (query.ts), and the client applies it to request parameters (client.ts). Yet TablesDB.listTables accepts only string[] in its parameter object and overloads (tables-db.ts). TypeScript callers therefore can’t use the builder directly even though the runtime handles it. Please widen the query parameter types and overloads to accept QueryInput.
|
All seven fixed in the generator (appwrite/sdk-generator#2046) and regenerated here. Map fields the spec defaults to |
|
@hansi-codes review |
|
@hansi-codes review |
| export { Role } from './role'; | ||
| export { ID } from './id'; | ||
| export { Topic, ResolvedTopic } from './topic'; | ||
| export { Topic } from './topic'; |
There was a problem hiding this comment.
Document the removed wildcard Topic API
This release removes Topic.any()/.all() and the ResolvedTopic export that were available in 30.0.0-rc.2, so existing wildcard-topic code now fails to compile or throws at runtime. Since the v30 breaking-changes list does not mention this removal, please retain the API or document this incompatibility.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 44
Comment:
**Document the removed wildcard Topic API**
This release removes `Topic.any()`/`.all()` and the `ResolvedTopic` export that were available in 30.0.0-rc.2, so existing wildcard-topic code now fails to compile or throws at runtime. Since the v30 breaking-changes list does not mention this removal, please retain the API or document this incompatibility.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
ChiragAgg5k
changed the title
|
@hansi-codes review |
| for (const key of Object.keys(model.defaults ?? {})) { | ||
| if (!Object.prototype.hasOwnProperty.call(value, key)) { | ||
| result[key] = cloneDefault(result[key]); | ||
| } |
There was a problem hiding this comment.
Add coverage for default filling and per-model copies
The new defaults path changes responses with omitted fields and is intended to give each model independent nested defaults, but the existing and added tests do not exercise either case. A focused test would catch regressions that can leave fields absent or let a mutation on one returned model affect another.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/client.ts
Line: 99-102
Comment:
**Add coverage for default filling and per-model copies**
The new defaults path changes responses with omitted fields and is intended to give each model independent nested defaults, but the existing and added tests do not exercise either case. A focused test would catch regressions that can leave fields absent or let a mutation on one returned model affect another.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · testing · Reply if this doesn't apply.
2 participants
This PR contains updates to the SDK for version 30.0.1.
What's Changed
X-Appwrite-Response-Format: 2.5.0; responses follow the 2.5 shapesaccount.listLogs,users.listLogsand theModels.Log/Models.LogListmodelsorganization.createKey,getKey,listKeys,updateKeyanddeleteKey, andOrganizationKeyScopes; useorganization.listProjectKeys,getProjectKey,updateProjectKeyanddeleteProjectKeyinsteadClient.setDevKey,Models.DevKeyandProject.devKeysBillingPlanGroup.Starter; useBillingPlanGroup.FreelistSpecificationsreturns each specification's prices and overage rates on the row itself. Removed the list-levelpricingfield and theDedicatedDatabaseSpecificationPricingmodelBlockfieldsreason,projectName,region,organizationName,organizationIdandbillingPlanresourceparams now takestring[]project.updateOAuth2KakaotakesenabledbeforepromptBackupRestoration.optionsis now an objectT | null; some became optional, such asBillingPlan.usageTopichelper to build MQTT topics for push messagesmongo,mysqlandpostgresqlpromoteRestoration,restoreMode,targetNameandautoPromoteoncreateRestoration, andDedicatedDatabaseRestoration.restoreModeproxy.createSMTPRule, theProxyRuleProtocolenum andProxyRule.protocolemailEnabledonfunctions.updateandFunction, andExecutionTrigger.EmailQueryaggregates (count,countDistinct,sum,avg,min,maxand more),groupBy,having,distinct, joins,relationship,covers,spatialEqualsandpageQuery.builder()to chain queries into a listnull, such asqoson topics and the Appwrite providerpromptand native fields,PlatformAndroid.fingerprintsandIdentity.providerIdTokenare always returned{}instead of[]