Update Serving API - from backend PR #5318 - #317
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @clllaur, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request updates the TypeScript Serving API definitions to synchronize with recent changes introduced in the backend repository. The changes primarily involve adding new types for file handling and optimistic concurrency, refining existing experience change types for their serving-specific representations, and making adjustments to the project configuration related to environments and global JavaScript code.
Highlights
- New Data Types: Introduced
UploadedFileDatato represent file objects with a storage key, original name, and URL, andConcurrencyKeyfor implementing optimistic concurrency control. - Optimistic Concurrency Support: Added an optional
concurrency_keyproperty to variousExperienceChangedata structures (e.g.,ExperienceChangeDefaultCodeData,ExperienceChangeDefaultRedirectData,ExperienceChangeCustomCodeData, etc.) and their corresponding update variants. This enables optimistic concurrency control for these entities when interacting with the API. - Serving-Specific Experience Change Types: Created new
*Servingtype variants (e.g.,ExperienceChangeDefaultCodeDataServing,ExperienceChangeRichStructureDataServing) for all experience change types. These*Servingtypes are specifically designed for the serving API and exclude theconcurrency_keyproperty. TheExperienceVariationConfigtype was updated to utilize these new serving-specific types for itschangesarray. - Project Configuration Adjustments: Removed the
environmentsproperty from theConfigProjecttype, indicating that environment details are no longer part of this specific configuration. Additionally, the JSDoc for theglobal_javascriptproperty withinConfigProjectwas clarified to explain how it combines with environment-specific code.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
There was a problem hiding this comment.
Code Review
This pull request updates the TypeScript types to align with recent backend API changes. The main changes include adding a concurrency_key for optimistic locking, introducing ...Serving type variants for API responses, and removing the environments property from the project configuration. The changes are generally sound, but I've identified several opportunities to improve the JSDoc comments for clarity and correctness, addressing issues like duplication, vagueness, and copy-paste errors. I've also suggested a minor stylistic improvement to a type alias.
| /** | ||
| * A server-generated hash that represents the object's state at the time of retrieval. When included in an update request, the operation will only succeed if the object hasn't been modified since this key was obtained. If another update has occurred in the meantime, the request will fail with a conflict error, requiring you to fetch the latest version and retry your update with the new concurrency_key. This implements optimistic concurrency control to prevent lost updates in concurrent scenarios. | ||
| */ | ||
| export type ConcurrencyKey = (string) | null; |
There was a problem hiding this comment.
The parentheses in (string) | null are redundant. For better readability and to follow common TypeScript conventions, it's better to write the union type directly.
| export type ConcurrencyKey = (string) | null; | |
| export type ConcurrencyKey = string | null; |
| /** | ||
| * Object that represents one change done inside an experience | ||
| */ |
There was a problem hiding this comment.
This JSDoc block is a duplicate of the one on lines 1125-1127 and appears to be redundant. Please remove it to improve code clarity.
| /** | ||
| * Describes structure for "defaultCode" type of experience change | ||
| */ |
There was a problem hiding this comment.
The JSDoc for ExperienceChangeDefaultCodeDataServing is identical to the one for ExperienceChangeDefaultCodeData. This pattern is repeated for all the new ...Serving types.
To improve clarity and maintainability, the comments should differentiate between the ...Data and ...DataServing types. For example, the ...Serving type is what's returned by the API for consumption, while the other is used for update payloads.
I suggest updating the comment to reflect this. Please apply a similar clarification to all other new ...Serving types in this file.
| /** | |
| * Describes structure for "defaultCode" type of experience change | |
| */ | |
| /** | |
| * Describes structure for "defaultCode" type of experience change, for serving purposes. | |
| */ |
| * Describes structure for "defaultCode" type of experience change | ||
| */ |
There was a problem hiding this comment.
The JSDoc here says this is for a "defaultCode" type of experience change, but the type is ExperienceChangeRichStructureData. This appears to be a copy-paste error and should be corrected to refer to "richStructure".
This error is repeated for other RichStructure and CustomCode types in this PR. Please correct them all.
Specifically:
ExperienceChangeRichStructureDataServing(line 1385)ExperienceChangeRichStructureDataUpdateNoId(line 1399)ExperienceChangeRichStructureDataUpdate(line 1406)ExperienceChangeCustomCodeData(line 1443)ExperienceChangeCustomCodeDataServing(line 1450)
| * Describes structure for "defaultCode" type of experience change | |
| */ | |
| * Describes structure for "richStructure" type of experience change | |
| */ |
3 participants
Updating TS Serving API after the latest changes from backend repo,
PR #5318