fix(sdk-client): omit undefined optional params from page.get() graphql.variables#36290
Conversation
…ql.variables (#36108) page.get() spread the optional request params (personaId, publishDate, variantName) into requestVariables without defaults, leaving `undefined` values in the returned graphql.variables object. That object is returned verbatim on the response and breaks JSON serialization for Next.js Pages Router consumers (getServerSideProps/getStaticProps throw on undefined). Only include these optional params when they are defined. The GraphQL request itself is unaffected — the variables are nullable and JSON.stringify already drops undefined keys. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @rjvelazco's task in 56s —— View job Rollback Safety Analysis
Result: ✅ Safe To RollbackThe label "AI: Safe To Rollback" has been applied. Diff scope: 4 files changed, all within Category-by-category analysis:
Summary: This PR only modifies a TypeScript SDK utility function ( |
🤖 Bedrock Review —
|
undefined optional params from page.get() graphql.variables
#36108) Replace the per-param undefined guards with a generic removeUndefinedValues helper applied to the whole requestVariables object. This protects any variable from leaking `undefined` (e.g. mode/languageId/fireRules if a caller passes them explicitly as undefined), not just the optional ones. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#36108) CI nx format:check flagged utils.ts. Collapse the single-statement helper signature/body to match the workspace prettier config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Tick the box to add this pull request to the merge queue (same as
|
…page.get() (dotCMS#36373) ## Proposed Changes Follow-up to dotCMS#36290 (already merged) addressing review feedback on dotCMS#36108. The first PR fixed the `undefined` values inside `graphql.variables`, but `page.get()` still leaked `undefined` in two other places that break Next.js Pages Router serialization (`getServerSideProps`/`getStaticProps` throw on `undefined`). ### Fixes 1. **`errors`** — returned `undefined` when there were no GraphQL errors. Now always returns an array (`[]` when none) so the response stays JSON-serializable. 2. **`errors` type** — `DotCMSPageResponse.errors` is now **required** (`errors: DotCMSGraphQLError[]`) to match the runtime contract, instead of `errors?`. The raw backend response type (`DotGraphQLApiResponse`) keeps `errors?` optional since the backend genuinely omits it on success. 3. **`pageAsset.page.styleEditorSchemas`** — `graphqlToPageEntity` set this to `undefined` when GraphQL returns `null` (outside EDIT_MODE) — a guaranteed `undefined` in every LIVE page. Now the key is omitted instead. Behavior is unchanged for populated/empty-array cases. 4. **Strengthened the regression test** to validate the **full response object** serializes (not just `graphql.variables`), so a top-level `undefined` field is caught next time. ###⚠️ Behavioral change (release notes) `page.get()` now always returns `errors` as an **array** (empty when none) instead of `undefined`. Consumers should check **`errors.length`**, not truthiness (`if (response.errors)` is now always truthy). This was explicitly requested in the issue feedback (`null` or `[]`). The required-type change is safe for anyone consuming `page.get()`'s return value (a stronger guarantee; `?.errors` still compiles). It only affects code that manually *constructs* a `DotCMSPageResponse` literal — verified `sdk-react`, `sdk-angular`, and `sdk-uve` still typecheck. ### Verification - All 287 `sdk-client` tests pass. - `sdk-client`, `sdk-react`, `sdk-angular`, `sdk-uve` build/typecheck clean. - Lint clean, format clean. - Verified the `core/examples` are unaffected — they branch on a thrown/caught singular `error` key, never the `errors` array. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Proposed Changes
Fixes #36108
client.page.get()returnedundefinedvalues insidegraphql.variables, breaking Next.js Pages Router apps that return the page response fromgetServerSideProps/getStaticProps(JSON has no representation forundefined).Root cause
In
core-web/libs/sdk/client/src/lib/client/page/page-api.ts, the optional request paramspersonaId,publishDate, andvariantNamewere destructured without defaults and spread verbatim intorequestVariables. That object is returned on the response, so theundefinedvalues surfaced to consumers.Fix
Only include the optional params (
personaId,publishDate,variantName) inrequestVariableswhen they are actually defined. The GraphQL request is unaffected — these variables are nullable andJSON.stringifyalready dropsundefinedkeys — so the only observable change is that the returnedgraphql.variablesobject is now JSON-serializable.Tests
personaId/publishDate/variantName: undefined.graphql.variablescontains noundefinedvalues and round-trips throughJSON.parse(JSON.stringify(...)).All 280
sdk-clienttests pass; lint clean.🤖 Generated with Claude Code
This PR fixes: #36108