feat: SDK update for version 1.1.0 - #118
Conversation
The badge already said 2.2.0; the sentence below it still claimed 2.0.x because the sdks task took the version from its --version argument (fixed in appwrite/appwrite#13784).
ChiragAgg5k
changed the title
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| static decodeBase64(data: string): Uint8Array { | ||
| const binary = atob(data); |
There was a problem hiding this comment.
atob, Expo fetch's call to the new file part's bytes() method throws here before the request is sent. This affects both small files and chunked uploads. Decode the base64 data without relying on that browser global.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/service.ts
Line: 43
Comment:
**Uploads depend on atob** If a React Native runtime does not provide a global `atob`, Expo fetch's call to the new file part's `bytes()` method throws here before the request is sent. This affects both small files and chunked uploads. Decode the base64 data without relying on that browser global.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Not introduced here: src/service.ts is not in this PR's diff. decodeBase64 and its atob call shipped in 1.0.0 (#117); this comment was made against a commit from before main was merged into dev. Hermes has provided a global atob since React Native 0.74, and this package requires react-native >=0.76.7, where Hermes is the default engine.
There was a problem hiding this comment.
You’re right on both points: src/service.ts is not part of PR #118, so this is an inherited issue from #117 rather than a regression introduced here. Given this package’s react-native >=0.76.7 peer requirement and Hermes providing atob in the supported runtime, the concern is not actionable for this PR. I’ll withdraw the comment.
| const result = await account.createIdTokenSession({ | ||
| provider: IdTokenProvider.Apple, | ||
| idToken: '<ID_TOKEN>', | ||
| nonce: '<NONCE>', // optional |
There was a problem hiding this comment.
| nonce: '<NONCE>', // optional | |
| nonce: '<NONCE>', |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/examples/account/create-id-token-session.md
Line: 13
Comment:
**Apple nonce marked optional** This example selects Apple, but the new API documentation says Apple sign-in requires a raw nonce. A developer who follows the optional label and omits it will have the ID token rejected.
```suggestion
nonce: '<NONCE>',
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (size <= Service.CHUNK_SIZE) { | ||
| payload['file'] = Service.filePart( | ||
| file.uri, | ||
| file.name, | ||
| file.type, | ||
| () => | ||
| FileSystem.readAsStringAsync(file.uri, { | ||
| encoding: FileSystem.EncodingType.Base64, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
FilePart or its configuration would not show whether the correct bytes reach the request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/storage.ts
Line: 290-299
Comment:
**Upload behavior lacks coverage** The new file-part path changes how both small files and upload chunks reach fetch, but there is no observable upload regression coverage. Add behavior-level checks for a small file and a multi-chunk file through the supported fetch paths. Tests that only mirror `FilePart` or its configuration would not show whether the correct bytes reach the request.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Not in this release: src/services/storage.ts is not in this PR's diff. The file-part upload path shipped in 1.0.0 (#117); this comment was made against a commit from before main was merged into dev.
| const apiPath = '/account/recovery/otp'; | ||
| const payload: Payload = {}; | ||
|
|
||
| if (typeof email !== 'undefined') { | ||
| payload['email'] = email; | ||
| } | ||
|
|
||
| if (typeof phrase !== 'undefined') { | ||
| payload['phrase'] = phrase; | ||
| } | ||
|
|
||
| const uri = new URL(this.client.config.endpoint + apiPath); | ||
| return this.client.call( | ||
| 'post', | ||
| uri, | ||
| { | ||
| 'X-Appwrite-Project': this.client.config.project, | ||
| 'content-type': 'application/json', | ||
| accept: 'application/json', | ||
| }, | ||
| payload, | ||
| ); |
There was a problem hiding this comment.
The new OTP initiation and completion requests have no observable-behavior tests. A regression in either step could go unnoticed until a user cannot reset their password. Please test successful recovery and rejected or expired codes through the SDK, rather than asserting that source code, headers, or version strings match fixed values.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/account.ts
Line: 2418-2439
Comment:
**Recovery flow lacks tests**
The new OTP initiation and completion requests have no observable-behavior tests. A regression in either step could go unnoticed until a user cannot reset their password. Please test successful recovery and rejected or expired codes through the SDK, rather than asserting that source code, headers, or version strings match fixed values.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
This PR contains updates to the SDK for version 1.1.0.
What's Changed
Account.createRecoveryOTPandAccount.updateRecoveryOTPfor code-based password recoveryX-Appwrite-Response-Format: 2.3.0)