-
Notifications
You must be signed in to change notification settings - Fork 21
feat: Dart SDK update for version 19.4.0 #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRefactors request construction across many services to omit null-valued parameters and centralizes explicit null-filtering in the client mixin. Adds a new Avatars.getScreenshot method with docs and a unit test. Introduces three new enums (Theme, Timezone, Output) and extends BuildRuntime/Runtime with dart-3.9 and flutter-3.35. Several endpoints now include width/height/quality query parameters only when provided. No public method signatures were removed; changes primarily affect how API payloads and query parameters are built and serialized. (≈70 words) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (3)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
test/services/avatars_test.dart (1)
146-157: LGTM!The test follows the established pattern for avatar service methods. Consider adding test cases that exercise some of the optional parameters (theme, timezone, output, etc.) to ensure proper parameter serialization.
lib/src/client_mixin.dart (1)
44-56: Consider filtering null elements from lists in GET parameters.The multipart handling (lines 27-33) explicitly filters out null elements from lists, but the GET parameter handling at line 50 assigns the entire list without filtering nulls. This inconsistency could result in null values being included in query parameters for GET requests.
Consider applying the same null-filtering logic:
if (value != null) { if (value is int || value is double) { filteredParams[key] = value.toString(); } else if (value is List) { - filteredParams["$key[]"] = value; + List nonNullValues = value.where((v) => v != null).toList(); + if (nonNullValues.isNotEmpty) { + filteredParams["$key[]"] = nonNullValues; + } } else { filteredParams[key] = value; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
CHANGELOG.md(1 hunks)docs/examples/avatars/get-screenshot.md(1 hunks)lib/enums.dart(1 hunks)lib/services/account.dart(7 hunks)lib/services/avatars.dart(6 hunks)lib/services/databases.dart(48 hunks)lib/services/functions.dart(13 hunks)lib/services/health.dart(14 hunks)lib/services/messaging.dart(47 hunks)lib/services/sites.dart(12 hunks)lib/services/storage.dart(9 hunks)lib/services/tables_db.dart(48 hunks)lib/services/teams.dart(4 hunks)lib/services/tokens.dart(1 hunks)lib/services/users.dart(18 hunks)lib/src/client_mixin.dart(2 hunks)lib/src/enums/build_runtime.dart(2 hunks)lib/src/enums/output.dart(1 hunks)lib/src/enums/runtime.dart(2 hunks)lib/src/enums/theme.dart(1 hunks)lib/src/enums/timezone.dart(1 hunks)test/services/avatars_test.dart(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/examples/avatars/get-screenshot.md
11-11: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (15)
CHANGELOG.md (1)
3-7: Verify version number consistency.The CHANGELOG indicates version 19.4.0, but the PR title states "version 19.3.0". Please confirm which version number is correct and ensure consistency across all documentation.
lib/src/enums/runtime.dart (1)
42-42: LGTM!The addition of
dart39andflutter335runtime versions follows the existing pattern and is consistent with the corresponding changes inBuildRuntime.Also applies to: 69-70
lib/src/enums/build_runtime.dart (1)
42-42: LGTM!The enum expansion mirrors the changes in
Runtime, maintaining consistency across build and runtime configurations.Also applies to: 69-70
lib/src/enums/timezone.dart (1)
1-429: LGTM!The
Timezoneenum is well-structured with comprehensive coverage of IANA timezone identifiers. The implementation follows the standard pattern with proper value mapping and JSON serialization support.lib/src/enums/output.dart (1)
1-17: LGTM!The
Outputenum properly defines supported image formats with the standard structure. The inclusion of bothjpgandjpegis appropriate for API compatibility.lib/enums.dart (1)
10-12: LGTM!The new part files are properly integrated into the public enum library, expanding the available enum types for SDK consumers.
lib/services/tokens.dart (1)
18-19: LGTM!The conditional parameter inclusion correctly implements the null-handling fix mentioned in the changelog. This pattern ensures only non-null optional parameters are included in the request body.
lib/src/enums/theme.dart (1)
3-12: LGTM!The
Themeenum is well-structured with proper value mapping and JSON serialization support.lib/services/messaging.dart (1)
13-17: LGTM!The conditional inclusion of optional parameters ensures cleaner API payloads by omitting null values. This pattern is applied consistently throughout the file.
lib/services/teams.dart (1)
14-18: LGTM!The conditional parameter inclusion pattern is correctly applied, ensuring optional query parameters are omitted when null.
lib/services/account.dart (1)
35-40: LGTM!The conditional inclusion of the optional
nameparameter is correct and aligns with the broader refactoring effort across the codebase.lib/src/client_mixin.dart (1)
19-40: LGTM!The null handling for multipart requests is thorough, correctly skipping null values at both the top level and within list elements.
lib/services/health.dart (1)
55-57: LGTM!The conditional inclusion of the optional
domainparameter follows the established pattern and correctly omits null values from the request.lib/services/users.dart (1)
13-17: LGTM!The conditional parameter inclusion pattern is consistently applied across all user management methods, ensuring cleaner API payloads.
lib/services/avatars.dart (1)
208-259: LGTM: New screenshot endpoint with comprehensive configuration options.The new
getScreenshotmethod provides extensive control over browser automation for webpage screenshots. The parameter handling follows the established null-safe pattern consistently.However, consider documenting any server-side constraints or validation rules for these parameters (e.g., maximum viewport dimensions, allowed permissions values, coordinate ranges for geolocation).
|
|
||
| Avatars avatars = Avatars(client); | ||
|
|
||
| UInt8List result = await avatars.getScreenshot( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the type name.
The return type should be Uint8List (lowercase 'i'), not UInt8List.
Apply this diff:
-UInt8List result = await avatars.getScreenshot(
+Uint8List result = await avatars.getScreenshot(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UInt8List result = await avatars.getScreenshot( | |
| Uint8List result = await avatars.getScreenshot( |
🤖 Prompt for AI Agents
In docs/examples/avatars/get-screenshot.md around line 10, the type name is
incorrect: change the return type from "UInt8List" to "Uint8List" (lowercase
'i') so the example uses the correct Dart type; update that occurrence
accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch details about the SDK generator PR
gh pr view 1253 --repo appwrite/sdk-generator --json title,body,state,url
Length of output: 133
@ChiragAgg5k, perfect! I can see you've already created PR #1253 to fix the typo at the generator level. This is exactly the right approach—fixing it at the source ensures all generated SDKs will have the correct Uint8List spelling going forward.
Once that PR is merged and you regenerate this SDK, the typo will be automatically corrected. Thanks for addressing this! 🎯
| viewportWidth: 1, // (optional) | ||
| viewportHeight: 1, // (optional) | ||
| scale: 0.1, // (optional) | ||
| theme: .light, // (optional) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the enum qualifier.
The theme parameter should be qualified with the enum name, e.g., Theme.light instead of .light.
Apply this diff:
- theme: .light, // (optional)
+ theme: Theme.light, // (optional)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| theme: .light, // (optional) | |
| theme: Theme.light, // (optional) |
🤖 Prompt for AI Agents
In docs/examples/avatars/get-screenshot.md around line 16, the theme parameter
uses an unqualified enum value ".light"; replace it with the fully-qualified
enum member (e.g., Theme.light) so the example compiles and reads correctly;
update the line to use Theme.light and ensure any other enum usages in this
snippet are similarly qualified.
| userAgent: '<USER_AGENT>', // (optional) | ||
| fullpage: false, // (optional) | ||
| locale: '<LOCALE>', // (optional) | ||
| timezone: .africaAbidjan, // (optional) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the enum qualifier.
The timezone parameter should be qualified with the enum name, e.g., Timezone.africaAbidjan instead of .africaAbidjan.
Apply this diff:
- timezone: .africaAbidjan, // (optional)
+ timezone: Timezone.africaAbidjan, // (optional)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| timezone: .africaAbidjan, // (optional) | |
| timezone: Timezone.africaAbidjan, // (optional) |
🤖 Prompt for AI Agents
In docs/examples/avatars/get-screenshot.md around line 20, the timezone enum
value is used without its qualifier; replace the unqualified `.africaAbidjan`
with the fully qualified enum `Timezone.africaAbidjan` (and ensure the Timezone
enum is imported or available in this example context if required).
| width: 0, // (optional) | ||
| height: 0, // (optional) | ||
| quality: -1, // (optional) | ||
| output: .jpg, // (optional) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the enum qualifier.
The output parameter should be qualified with the enum name, e.g., Output.jpg instead of .jpg.
Apply this diff:
- output: .jpg, // (optional)
+ output: Output.jpg, // (optional)🤖 Prompt for AI Agents
In docs/examples/avatars/get-screenshot.md around line 30, the example sets
output: .jpg but must use the enum qualifier; change the value to Output.jpg (or
the correct enum namespace used in this project) so the example reads output:
Output.jpg, and if the enum is in a different module ensure the example
references it correctly (add the proper import or namespace prefix as used
elsewhere).
This PR contains updates to the Dart SDK for version 19.4.0.
Summary by CodeRabbit
New Features
getScreenshot()method to capture web page screenshots.Theme,Output, andTimezonepublic enums for configuration.Bug Fixes