refactor(google): readonly-domain-types (stack 3/12, re-split #307) - #376
Conversation
|
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:M
PR Summary by QodoGoogle: adapt Play clients to readonly domain types via mutable request/normalize shapes
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1. Unchecked request cast
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| /** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */ | ||
| const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => { | ||
| const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape)); | ||
| return clonedRequest; |
There was a problem hiding this comment.
1. Unchecked request cast 🐞 Bug ⚙ Maintainability
mutableGoogleRequest accepts unknown but returns the caller-selected generic type, so request bodies are no longer type-checked against the googleapis schema types at the call site. This can allow an incompatible request shape to compile and only fail at runtime in write paths (tracks/subscriptions/offers).
Agent Prompt
### Issue description
`mutableGoogleRequest` currently has the signature `<T>(requestShape: unknown) => T`, which turns it into an unchecked cast: callers can request any return type regardless of the actual argument shape.
### Issue Context
This helper is used to build `requestBody` objects for googleapis write calls (tracks/subscriptions/offers). Keeping the deep-clone behavior is fine, but we should preserve compiler checking that the provided object is actually assignable to the intended Google schema.
### Fix Focus Areas
- src/google/playClient.ts[10-13]
- src/google/playClient.ts[656-668]
- src/google/playClient.ts[883-896]
- src/google/playClient.ts[965-985]
### Suggested fix
1. Change the helper to be type-safe at the input:
- e.g. `const mutableGoogleRequest = <T>(requestShape: T): MutableDeep<T> => structuredClone(requestShape) as MutableDeep<T>;`
- (If `structuredClone` is not desired, keep JSON clone but still type the parameter as `T`, not `unknown`, and cast internally.)
2. At call sites, make the argument satisfy the Google schema type so TS checks it:
- `requestBody: mutableGoogleRequest({ ... } satisfies androidpublisher_v3.Schema$Subscription)`
- or assign the object to a `const body: androidpublisher_v3.Schema$Subscription = { ... }` before cloning.
This preserves the “mutable plain object” goal while preventing accidental schema drift from compiling silently.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => { | ||
| const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape)); | ||
| return clonedRequest; |
There was a problem hiding this comment.
2. Brittle json deep clone 🐞 Bug ☼ Reliability
mutableGoogleRequest deep-clones via JSON.parse(JSON.stringify(...)), which will throw or change values if non-JSON-safe data ever reaches a request body (e.g., circular references, BigInt, or undefined array entries becoming null). That creates a sharp edge on the Google write paths that now always route through this helper.
Agent Prompt
### Issue description
The helper clones request bodies using JSON serialization, which is inherently lossy and can throw for certain runtime values.
### Issue Context
Even if current request bodies are intended to be JSON-shaped, this helper is now the standard pathway for several write operations; using a non-lossy clone reduces fragility if the shapes evolve.
### Fix Focus Areas
- src/google/playClient.ts[10-13]
### Suggested fix
Replace JSON cloning with `structuredClone` (Node >= 20) to avoid JSON lossy behavior:
- `const clonedRequest = structuredClone(requestShape) as MutableDeep<T>;`
If you must keep JSON cloning, consider adding an explicit error message when stringify/parse fails so failures are more diagnosable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/google/playClient.ts">
<violation number="1" location="src/google/playClient.ts:11">
P2: mutableGoogleRequest takes `requestShape: unknown` and returns the caller-supplied generic type, effectively acting as an unchecked cast. Callers can pass any object shape and specify any return type (e.g. `mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({...})`), and TypeScript won't verify that the argument actually matches the requested schema. This removes compile-time safety on write paths for tracks/subscriptions/offers, allowing schema drift to compile and fail only at runtime. Consider typing the parameter as `T` (or using `satisfies` at call sites) so the compiler still verifies the input shape.</violation>
<violation number="2" location="src/google/playClient.ts:12">
P2: The new `mutableGoogleRequest` uses a `JSON.parse(JSON.stringify(...))` round-trip to produce a mutable request body for Google's generated client. Today the inputs (normalized Play releases/subscriptions/offers) are plain JSON-safe data, so nothing breaks, but this clone is lossy and unguarded: it throws a `SyntaxError` if the input is ever `undefined`, and it silently drops `undefined` fields and cannot represent `Date`/`BigInt`/circular values. Since the only requirement is satisfying the mutable client types, a `structuredClone` (which preserves values without the JSON loss) is safer; if a plain cast is acceptable that avoids the clone cost entirely.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| /** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */ | ||
| const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => { | ||
| const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape)); |
There was a problem hiding this comment.
P2: The new mutableGoogleRequest uses a JSON.parse(JSON.stringify(...)) round-trip to produce a mutable request body for Google's generated client. Today the inputs (normalized Play releases/subscriptions/offers) are plain JSON-safe data, so nothing breaks, but this clone is lossy and unguarded: it throws a SyntaxError if the input is ever undefined, and it silently drops undefined fields and cannot represent Date/BigInt/circular values. Since the only requirement is satisfying the mutable client types, a structuredClone (which preserves values without the JSON loss) is safer; if a plain cast is acceptable that avoids the clone cost entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/google/playClient.ts, line 12:
<comment>The new `mutableGoogleRequest` uses a `JSON.parse(JSON.stringify(...))` round-trip to produce a mutable request body for Google's generated client. Today the inputs (normalized Play releases/subscriptions/offers) are plain JSON-safe data, so nothing breaks, but this clone is lossy and unguarded: it throws a `SyntaxError` if the input is ever `undefined`, and it silently drops `undefined` fields and cannot represent `Date`/`BigInt`/circular values. Since the only requirement is satisfying the mutable client types, a `structuredClone` (which preserves values without the JSON loss) is safer; if a plain cast is acceptable that avoids the clone cost entirely.</comment>
<file context>
@@ -5,6 +5,13 @@ import {
+
+/** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */
+const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => {
+ const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape));
+ return clonedRequest;
+};
</file context>
| import type { MutableDeep } from '../core/types/mutable.js'; | ||
|
|
||
| /** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */ | ||
| const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => { |
There was a problem hiding this comment.
P2: mutableGoogleRequest takes requestShape: unknown and returns the caller-supplied generic type, effectively acting as an unchecked cast. Callers can pass any object shape and specify any return type (e.g. mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({...})), and TypeScript won't verify that the argument actually matches the requested schema. This removes compile-time safety on write paths for tracks/subscriptions/offers, allowing schema drift to compile and fail only at runtime. Consider typing the parameter as T (or using satisfies at call sites) so the compiler still verifies the input shape.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/google/playClient.ts, line 11:
<comment>mutableGoogleRequest takes `requestShape: unknown` and returns the caller-supplied generic type, effectively acting as an unchecked cast. Callers can pass any object shape and specify any return type (e.g. `mutableGoogleRequest<androidpublisher_v3.Schema$Subscription>({...})`), and TypeScript won't verify that the argument actually matches the requested schema. This removes compile-time safety on write paths for tracks/subscriptions/offers, allowing schema drift to compile and fail only at runtime. Consider typing the parameter as `T` (or using `satisfies` at call sites) so the compiler still verifies the input shape.</comment>
<file context>
@@ -5,6 +5,13 @@ import {
+import type { MutableDeep } from '../core/types/mutable.js';
+
+/** Deep-clone a Launch boundary value into a mutable plain object for Google's generated client. */
+const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => {
+ const clonedRequest: GoogleRequest = JSON.parse(JSON.stringify(requestShape));
+ return clonedRequest;
</file context>
| const mutableGoogleRequest = <GoogleRequest>(requestShape: unknown): GoogleRequest => { | |
| const mutableGoogleRequest = <GoogleRequest>(requestShape: GoogleRequest): MutableDeep<GoogleRequest> => { | |
| const clonedRequest: MutableDeep<GoogleRequest> = JSON.parse(JSON.stringify(requestShape)); | |
| return clonedRequest; | |
| }; |
|
Superseded by land of tip stack #386 (same 12 domain commits). |
1 participant
User description
Stack 3/12 of re-split HOLD #307
Domain:
googleBase:
refactor/types/readonly-stack-02-appleFull green tip:
refactor/foundation/readonly-types-fullLand stack in order. Intermediate PRs may not typecheck alone.
Summary by cubic
Refactors the Google Play clients to work with readonly domain types by constructing mutable objects at API boundaries. Adds a deep-clone helper for request bodies to satisfy the
@googleapis/androidpublisherclient.mutableGoogleRequest<T>()and applied it to track, subscription (create/update), and offer requests so the Google client receives plain mutable objects.MutableDeepmodels throughout the Play client and reporting (e.g., reviews, releases, products, base plans, subscriptions, offers, replies, availability, converted prices, vitals rows).Written for commit 64a71e5. Summary will update on new commits.
CodeAnt-AI Description
Keep Google Play operations compatible with read-only domain data
What Changed
Impact
✅ Reliable Google Play catalog updates✅ Read-only data remains safe during API requests✅ Stable subscription and reporting reconciliation💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.