Skip to content

Sanitize page.get GraphQL variables for JSON-safe SSR responses #36208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
+43 −19
Draft

Sanitize page.get GraphQL variables for JSON-safe SSR responses #36208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f018c0e
Initial plan
Copilot Jun 17, 2026
4b11436
fix: omit undefined page graphql vars
Copilot Jun 17, 2026
d29e4a9
docs: clarify page graphql variable sanitizing
Copilot Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions core-web/libs/sdk/client/src/lib/client/page/page-api.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,7 @@ describe('PageClient', () => {
mode: 'LIVE',
languageId: '1',
fireRules: false,
siteId: 'test-site',
personaId: undefined,
publishDate: undefined,
variantName: undefined
siteId: 'test-site'
}
}
});
Expand Down Expand Up @@ -355,10 +352,7 @@ describe('PageClient', () => {
mode: 'LIVE',
languageId: '1',
fireRules: false,
siteId: 'test-site',
personaId: undefined,
publishDate: undefined,
variantName: undefined
siteId: 'test-site'
});
}
}
Expand Down Expand Up @@ -591,7 +585,6 @@ describe('PageClient', () => {
fireRules: false,
siteId: 'test-site',
personaId: 'test-persona',
publishDate: undefined,
variantName: 'test-variant',
customVar: 'customValue'
});
Expand Down Expand Up @@ -920,10 +913,7 @@ describe('PageClient', () => {
mode: 'LIVE',
languageId: '1',
fireRules: false,
siteId: 'test-site',
personaId: undefined,
publishDate: undefined,
variantName: undefined
siteId: 'test-site'
});
});

Expand Down Expand Up @@ -1222,6 +1212,29 @@ describe('PageClient', () => {
expect(result.graphql.variables).toEqual(getRequestBody().variables);
});

it('omits undefined optional variables from the returned graphql.variables', async () => {
const pageClient = new PageClient(
validConfig,
requestOptions,
new FetchHttpClient()
);
const result = await pageClient.get('/home');

expect(result.graphql.variables).toEqual({
url: '/home',
mode: 'LIVE',
languageId: '1',
fireRules: false,
siteId: 'test-site'
});
expect(Object.values(result.graphql.variables).every((value) => value !== undefined)).toBe(
true
);
expect(JSON.parse(JSON.stringify(result.graphql.variables))).toEqual(
result.graphql.variables
);
});

it('result.errors is undefined when response has no errors', async () => {
const pageClient = new PageClient(
validConfig,
Expand Down
23 changes: 17 additions & 6 deletions core-web/libs/sdk/client/src/lib/client/page/page-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ function logVerboseError(
);
}

/**
* Removes undefined values so returned GraphQL metadata stays JSON-serializable
* for consumers such as Next.js Pages Router.
*/
function omitUndefinedValues(variables: Record<string, unknown>): Record<string, unknown> {
return Object.fromEntries(
Object.entries(variables).filter(([, value]) => value !== undefined)
);
}

/**
* Client for interacting with the DotCMS Page API.
* Provides methods to retrieve and manipulate pages.
Expand Down Expand Up @@ -160,6 +170,7 @@ export class PageClient extends BaseApiClient {

const requestHeaders = this.requestOptions.headers;
const requestBody = JSON.stringify({ query: completeQuery, variables: requestVariables });
const responseVariables = omitUndefinedValues(requestVariables);

try {
const response = await fetchGraphQL({
Expand Down Expand Up @@ -203,7 +214,7 @@ export class PageClient extends BaseApiClient {
message: firstError?.message ?? 'GraphQL query failed',
data: response.errors
}),
{ query: completeQuery, variables: requestVariables }
{ query: completeQuery, variables: responseVariables }
);
}

Expand Down Expand Up @@ -239,7 +250,7 @@ export class PageClient extends BaseApiClient {

throw new DotErrorPage(message, status, code, undefined, {
query: completeQuery,
variables: requestVariables
variables: responseVariables
});
}
}
Expand All @@ -262,7 +273,7 @@ export class PageClient extends BaseApiClient {
message: `Page '${normalizedUrl}' was not found`,
data: response.errors
}),
{ query: completeQuery, variables: requestVariables }
{ query: completeQuery, variables: responseVariables }
);
}

Expand All @@ -274,7 +285,7 @@ export class PageClient extends BaseApiClient {
content: contentResponse,
graphql: {
query: completeQuery,
variables: requestVariables
variables: responseVariables
},
errors: response.errors?.length ? response.errors : undefined,
...(styleEditorSchemas?.length && { styleEditorSchemas })
Expand All @@ -290,7 +301,7 @@ export class PageClient extends BaseApiClient {
error.status,
'UNKNOWN',
error,
{ query: completeQuery, variables: requestVariables }
{ query: completeQuery, variables: responseVariables }
);
}

Expand All @@ -299,7 +310,7 @@ export class PageClient extends BaseApiClient {
500,
'UNKNOWN',
undefined,
{ query: completeQuery, variables: requestVariables }
{ query: completeQuery, variables: responseVariables }
);
}
}
Expand Down