P3.1: generate TypeScript SDK prerelease - #2
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6ed8a829c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'partnerSkuId': json['partner_sku_id'] == null ? undefined : json['partner_sku_id'], | ||
| 'firmwareReleaseId': json['firmware_release_id'] == null ? undefined : json['firmware_release_id'], | ||
| 'name': json['name'] == null ? undefined : json['name'], |
There was a problem hiding this comment.
Preserve null values during model conversion
When an API response explicitly contains null for partner_sku_id, firmware_release_id, or name, these conversions replace it with undefined, despite the corresponding properties being declared as nullable. This loses the distinction between a returned null and an omitted field, and round-tripping through DeviceToJSON subsequently omits the property entirely; preserve null when the JSON key is present and null.
Useful? React with 👍 / 👎.
| */ | ||
|
|
||
|
|
||
| export const BASE_PATH = "https://api.corelink.example".replace(/\/+$/, ""); |
There was a problem hiding this comment.
Require a usable API base path
When an API class is constructed without an explicit Configuration, every request uses https://api.corelink.example; the reserved .example domain cannot reach a CoreLink deployment, so the default client always fails before receiving a response. Either provide the actual public endpoint or make basePath required and document its value rather than exposing a silently unusable default.
Useful? React with 👍 / 👎.
| pipes: "|", | ||
| }; | ||
|
|
||
| export type FetchAPI = WindowOrWorkerGlobalScope['fetch']; |
There was a problem hiding this comment.
Use a Node-compatible fetch type
In a Node TypeScript project that excludes the DOM libraries, importing this package's declarations fails because WindowOrWorkerGlobalScope is defined only by TypeScript's DOM/WebWorker libraries; Node's fetch typings do not supply that interface. This prevents the SDK from type-checking in otherwise valid Node fetch environments, so FetchAPI should be expressed using a runtime-neutral function signature or Node-compatible global fetch type.
Useful? React with 👍 / 👎.
| } else if (this.isJsonMime(headers['Content-Type'])) { | ||
| body = JSON.stringify(overriddenInit.body); |
There was a problem hiding this comment.
Preserve pre-serialized bodies supplied by request overrides
When initOverrides replaces a generated request body with an already serialized string while retaining a JSON content type, this branch calls JSON.stringify on that string and sends a quoted, double-encoded payload. The same logic can turn other valid RequestInit.body values into {} because it bases serialization solely on the content type; body overrides should be sent unchanged when they are already valid BodyInit values.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c001aed7e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ready. | ||
| - Contract: `corelink-public-v1.yaml`, version `1.0.0-draft` | ||
| - Source branch: `agent/p3-1-contracts` | ||
| - Source commit: `701e693` |
There was a problem hiding this comment.
Align the README with the recorded contract commit
The README identifies 701e693 as the source contract commit, while both .corelink-contract.json and CODEGEN.md record 2fb0dc8 as the authoritative input. This makes the generated SDK's provenance ambiguous and prevents consumers or release automation from reliably reproducing or validating the advertised contract; update the README to reference the actual generation commit.
Useful? React with 👍 / 👎.
| }; | ||
| } | ||
|
|
||
| export function DeviceToJSON(json: any): Device { |
There was a problem hiding this comment.
Type JSON serializers as serialized objects
DeviceToJSON is publicly exported with a return type of Device, but the implementation returns a wire-format object whose properties are snake_case, such as corelink_device_id, rather than the camelCase Device interface. Consequently, consumer code can type-check while reading serialized.corelinkDeviceId even though that property is absent at runtime; the generated *ToJSON helpers should return an accurate wire-format type or any instead of their input model type.
Useful? React with 👍 / 👎.
Part of CoreLinkPlatform/platform#94; depends on CoreLinkPlatform/api-contracts#2 and CoreLinkPlatform/platform#144.
Generated with OpenAPI Generator 7.12.0 from
corelink-public-v1.yamlat contract commit701e693.Validation:
npm install && npm run test:generatedpassed; npm audit reported 0 vulnerabilities.