Skip to content

Conversation

@ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Nov 19, 2025

Summary

This PR adds support for generating types for geometric attributes (POINT, LINESTRING, POLYGON) across all supported languages in the CLI type generation feature.

Changes

  • Added POINT, LINESTRING, and POLYGON to the AttributeType enum in attribute.js.twig
  • Implemented type mapping logic for all language generators:
    • TypeScript: Array<number>, Array<Array<number>>, Array<Array<Array<number>>>
    • JavaScript: number[], number[][], number[][][]
    • C#: List<double>, List<List<double>>, List<List<List<double>>>
    • Dart: List<double>, List<List<double>>, List<List<List<double>>>
    • Java: List<Double>, List<List<Double>>, List<List<List<Double>>>
    • Kotlin: List<Double>, List<List<Double>>, List<List<List<Double>>>
    • PHP: array (for all geometric types)
    • Swift: [Double], [[Double]], [[[Double]]]

Type Structure

  • POINT: Array of numbers representing coordinates [longitude, latitude]
  • LINESTRING: Array of points (array of arrays) representing a line
  • POLYGON: Array of linestrings (array of array of arrays) representing a polygon with optional holes

Test plan

  • Verify type generation works correctly for POINT attributes
  • Verify type generation works correctly for LINESTRING attributes
  • Verify type generation works correctly for POLYGON attributes
  • Test with all supported languages (TypeScript, JavaScript, C#, Dart, Java, Kotlin, PHP, Swift)

Summary by CodeRabbit

  • New Features

    • Added support for geometric attribute types: POINT, LINESTRING, POLYGON across code generators — mapped to nested numeric array types in generated SDKs.
  • Chores

    • Updated development tooling in templates (Playwright version bumped).

✏️ Tip: You can customize this high-level summary in your review settings.

…olygon)

This commit adds support for generating types for geometric attributes (POINT, LINESTRING, POLYGON) across all supported languages in the CLI type generation feature.

Type mappings:
- TypeScript/JavaScript: Array<number>, Array<Array<number>>, Array<Array<Array<number>>>
- C#/Dart/Java/Kotlin: List<double>/List<List<double>>/List<List<List<double>>>
- PHP: array (for all geometric types)
- Swift: [Double], [[Double]], [[[Double]]]

Changes include:
- Added POINT, LINESTRING, POLYGON to AttributeType enum
- Implemented type mapping logic for all language generators
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Walkthrough

This PR adds three geometry attribute types to the AttributeType enumeration: POINT, LINESTRING, POLYGON. It updates type-generation templates for C#, Dart, Java, JavaScript, Kotlin, PHP, Swift, and TypeScript to map these types to nested numeric arrays/lists (increasing nesting depth from POINT → LINESTRING → POLYGON). It updates two template package.json files (react-native and web) to bump devDependency "playwright" from 1.15.0 to 1.56.1. example.php now introduces a $branch variable and fetches the Swagger spec using the branch path. tests/WebChromiumTest.php updates Playwright Docker image tags to v1.56.1-jammy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas needing attention:

  • Consistency of nesting depth across all language templates (POINT: 1D, LINESTRING: 2D, POLYGON: 3D).
  • PHP mapping uses generic array — confirm intended shape or explicit nested typing.
  • Ensure additions to AttributeType enum don't break downstream consumers or serialization.
  • Verify Playwright version bump and Docker tag changes (v1.15.0 → v1.56.1-jammy) are intentional and compatible with CI/test scripts.
  • Confirm example.php branch change is intentional and secure for fetching specs.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main change: adding type generation support for geometric types (POINT, LINESTRING, POLYGON) across multiple language generators.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-add-geometric-types-support

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 099bfbd and ae5294d.

📒 Files selected for processing (4)
  • example.php (1 hunks)
  • templates/react-native/package.json.twig (1 hunks)
  • templates/web/package.json.twig (1 hunks)
  • tests/WebChromiumTest.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • templates/web/package.json.twig
  • templates/react-native/package.json.twig
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: build (8.3, WebNode)
  • GitHub Check: build (8.3, Python311)
  • GitHub Check: build (8.3, Python310)
  • GitHub Check: build (8.3, PHP80)
  • GitHub Check: kotlin (server)
  • GitHub Check: build (8.3, KotlinJava17)
  • GitHub Check: swift (server)
  • GitHub Check: build (8.3, DotNet90)
  • GitHub Check: build (8.3, FlutterStable)
  • GitHub Check: flutter (client)
  • GitHub Check: build (8.3, FlutterBeta)
  • GitHub Check: build (8.3, Go118)
  • GitHub Check: build (8.3, KotlinJava8)
  • GitHub Check: build (8.3, DartBeta)
  • GitHub Check: android (client)
  • GitHub Check: apple (client)
  • GitHub Check: build (8.3, CLINode16)
  • GitHub Check: build (8.3, Android5Java17)
  • GitHub Check: build (8.3, Android14Java17)
🔇 Additional comments (2)
example.php (1)

97-99: The original review comment is based on a misunderstanding of the code's intent.

The separation of $branch and $version was intentional, as confirmed by git commit d7ac737 ("seperate out branch in example file"). While both currently have the value '1.8.x', they serve distinct purposes in the URL structure:

  • {$branch} references the GitHub repository branch (appwrite/{$branch}/...)
  • {$version} identifies the specific spec file version (swagger2-{$version}-...)

This design allows these values to be managed independently in the future, even if they currently match. The identical values do not indicate redundancy but rather a deliberate architectural separation.

Likely an incorrect or invalid review comment.

tests/WebChromiumTest.php (1)

18-21: Docker image tag is correct; verify test code for breaking changes between Playwright versions

The Docker image mcr.microsoft.com/playwright:v1.56.1-jammy is the official Playwright v1.56.1 image and is correctly specified. However, the jump from v1.15.0 to v1.56.1 introduces several breaking changes that may affect tests.js:

  • page.route() glob patterns no longer support ? and [] (use RegExp instead)
  • route.continue() can no longer override Cookie headers (use browserContext.addCookies() instead)
  • page.type() / locator.type() deprecated (use locator.fill() instead)
  • browserContext.on('backgroundpage') is deprecated
  • Browser binary and headless behavior changes may affect rendering/timing

Review your test code to ensure it doesn't rely on these deprecated or changed APIs before merging.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for generating types for geometric database attributes (POINT, LINESTRING, POLYGON) across all supported programming languages in the CLI type generation feature. The changes enable developers to work with spatial/geographic data types in their generated type definitions.

Key changes:

  • Extended the AttributeType enum with three new geometric types
  • Implemented appropriate type mappings for all eight supported language generators
  • Ensured consistent representation of nested array structures for each geometric type

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
templates/cli/lib/type-generation/attribute.js.twig Added POINT, LINESTRING, and POLYGON constants to the AttributeType enum
templates/cli/lib/type-generation/languages/typescript.js.twig Added type mappings using Array syntax for geometric types
templates/cli/lib/type-generation/languages/javascript.js.twig Added type mappings using number[] JSDoc syntax for geometric types
templates/cli/lib/type-generation/languages/csharp.js.twig Added type mappings using List for geometric types
templates/cli/lib/type-generation/languages/dart.js.twig Added type mappings using List for geometric types
templates/cli/lib/type-generation/languages/java.js.twig Added type mappings using List for geometric types
templates/cli/lib/type-generation/languages/kotlin.js.twig Added type mappings using List for geometric types
templates/cli/lib/type-generation/languages/php.js.twig Added type mappings using array for all geometric types
templates/cli/lib/type-generation/languages/swift.js.twig Added type mappings using [Double] syntax for geometric types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
templates/react-native/package.json.twig (1)

29-29: Same playwright version concern as web template.

This change has the same issues noted in templates/web/package.json.twig line 29: the playwright version bump appears unrelated to the PR's geometric type objectives and should be verified for validity and security.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eea14a6 and 099bfbd.

📒 Files selected for processing (2)
  • templates/react-native/package.json.twig (1 hunks)
  • templates/web/package.json.twig (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: build (8.3, Python39)
  • GitHub Check: build (8.3, PHP83)
  • GitHub Check: build (8.3, Python310)
  • GitHub Check: build (8.3, Node18)
  • GitHub Check: build (8.3, KotlinJava17)
  • GitHub Check: build (8.3, KotlinJava11)
  • GitHub Check: build (8.3, Node16)
  • GitHub Check: build (8.3, KotlinJava8)
  • GitHub Check: build (8.3, DotNet90)
  • GitHub Check: build (8.3, DotNet80)
  • GitHub Check: build (8.3, Android5Java17)
  • GitHub Check: build (8.3, FlutterBeta)
  • GitHub Check: build (8.3, Android14Java17)
  • GitHub Check: build (8.3, FlutterStable)
  • GitHub Check: kotlin (server)
  • GitHub Check: swift (server)
  • GitHub Check: flutter (client)
  • GitHub Check: android (client)
  • GitHub Check: apple (client)

@ChiragAgg5k ChiragAgg5k force-pushed the feat-add-geometric-types-support branch from 0bf0293 to 734f44a Compare November 20, 2025 05:01
@Meldiron Meldiron merged commit 05367bc into master Nov 20, 2025
65 of 67 checks passed
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.

3 participants