Issue 4359 pretty print response #7634

dfperry5 · 2023-07-08T14:53:16Z

This is a small Pull Request that addresses Issue #4359.

It adds a new function stringifyResults(FormattedExecutionResult) => string to the ApolloServerOptionsBase and ApolloServerInternals. This allows consumers to pass in a function that manipulates the FormattedExecutionResult into a string of their preferred format and liking. This function is used within the runHttpQuery function. If the stringifyResult attribute is present, we will pass the FormattedExecutionResult to the supplied function, and its returned value will be the string portion of the HTTPGraphQLResponse.body. If the stringifyResult function is not provided, we fall back to the default behavior of prettyJSONStringify(orderExecutionResultFields(graphQLResponse.body.singleResult).

…sult in the http query response

apollo-cla · 2023-07-08T14:53:18Z

@dfperry5: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

netlify · 2023-07-08T14:53:18Z

👷 Deploy request for apollo-server-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	`337b432`

codesandbox-ci · 2023-07-08T14:54:07Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 337b432:

Sandbox	Source
Apollo Server Typescript	Configuration
Apollo Server	Configuration

…5/apollo-server into Issue-4359-Pretty-Print-Response

dfperry5 · 2023-07-08T15:18:49Z

Apologies for all the clean up commits - but, I do have a question:

Should I be running npm run changeset-publish? I couldn't find a reference to it in the contributing.md file.

trevor-scheer

No need to do any publishing, but you can add a changeset by running npx changeset. This should be a minor change. Please include a description of the change that's useful for users. Code snippets / example usage are great to have!

trevor-scheer · 2023-07-10T17:14:29Z

packages/integration-testsuite/src/httpServerTests.ts

+      it('uses stringifyResult parameter', async () => {
+        const app = await createApp({
+          schema,
+          stringifyResult: (value: FormattedExecutionResult) => {
+            let result = JSON.stringify(value, null, 1); // stringify, with line-breaks and indents
+            result = result.replace('it works', 'stringifyResults works!'); // replace text with something custom
+            return result;
+          },
+        });
+        const expected = {
+          testString: 'stringifyResults works!',
+        };
+        const query = {
+          query: 'query test{ testString }',
+        };
+        const req = request(app)
+          .get('/')
+          .set('apollo-require-preflight', 't')
+          .query(query);
+        return req.then((res) => {
+          expect(res.status).toEqual(200);
+          expect(res.body.data).toEqual(expected);
+        });
+      });


I think this is specific to Apollo Server and doesn't necessarily need to be tested for all integrations. I think this might belong in ApolloServer.test.ts instead under a describe('configuration options', ...) block.

There seems to be a couple layers in testing this. Your replace is clearly working as expected but the additional JSON.stringify formatting arguments are of no consequence to this test (i.e. I can remove them and the test still passes) so I'd rather see a test that validates the actual string output (probably with toMatchInlineSnapshot) or something similar.

Sidenote: res.body.data is a parsed object, so it seems like res.text would be more appropriate for validating the formatting changes.

trevor-scheer · 2023-07-10T17:16:20Z

No worries on the commits, they'll all be squashed into one when we land the PR.

dfperry5 · 2023-07-10T22:10:10Z

@trevor-scheer -- Updated :) Let me know if i misunderstood anything.

trevor-scheer · 2023-07-10T22:19:05Z

.changeset/pink-jobs-accept.md

+        const server = new ApolloServer({
+          typeDefs,
+          resolvers,
+          stringifyResult: (value: FormattedExecutionResult) => {
+            let result = JSON.stringify(value, null, 10000) // modify response
+            result = result.replace('world', 'stringifyResults works!'); // replace text with something custom
+            return result;
+          },
+        });


I can't use the suggest feature here due to character escaping but if you could surround the example with code fences for markdown readers that would be nice.

```ts
code example
```

Maybe for the example we just provide the most common transform people use for pretty print? Not sure the replacement is going to be a common use case.

return JSON.stringify(value, null, 2);

trevor-scheer · 2023-07-10T22:21:10Z

packages/integration-testsuite/src/httpServerTests.ts

@@ -2060,6 +2061,33 @@ export function defineIntegrationTestSuiteHttpServerTests(
          expect(res.body.errors[0].message).toEqual(expected);
        });
      });
+
+      it('uses stringifyResult parameter', async () => {


We can just remove this test altogether with the existence of the other test. It looks like there are a handful of snapshot files that were generated due to using toMatchSnapshot instead of toMatchInlineSnapshot - we can delete those too.

trevor-scheer · 2023-07-10T22:22:10Z

npm run prettier-fix to get the prettier check passing

…ting changeset file

dfperry5 · 2023-07-10T23:29:06Z

Updated

trevor-scheer

Just a few last minor tweaks and this is good to go!

trevor-scheer · 2023-07-10T23:18:43Z

.changeset/pink-jobs-accept.md

+  typeDefs,
+  resolvers,
+  stringifyResult: (value: FormattedExecutionResult) => {
+    return JSON.stringify(value, null, 2);;


Suggested change

return JSON.stringify(value, null, 2);;

return JSON.stringify(value, null, 2);

Yikes! :P Good catch.

trevor-scheer · 2023-07-10T23:20:08Z

packages/server/src/__tests__/ApolloServer.test.ts

@@ -141,6 +141,40 @@ describe('ApolloServer construction', () => {
    // @ts-expect-error
    takesConfig({ modules: [] });
  });
+  describe('with configuration options', () => {
+    it('test', async () => {


Suggested change

it('test', async () => {

it('stringifyResult', async () => {

sigh - sorry this was a dumb one. Rushing through it!

trevor-scheer · 2023-07-10T23:28:25Z

packages/server/src/__tests__/ApolloServer.test.ts

+      const { body } = await server.executeHTTPGraphQLRequest(request);
+      expect(JSON.stringify(body)).toMatchInlineSnapshot(
+        `"{\"kind\":\"complete\",\"string\":\"{\\n          \\\"data\\\": {\\n                    \\\"hello\\\": \\\"stringifyResults works!\\\"\\n          }\\n}\"}"`,
+      );
+      // const result = singleResult(body);
+      // expect(result.errors).toBeUndefined();
+      // expect(result.data?.hello).toBe('world');


It's a little weird to stringify the response body for snapshotting, I think this is what you're looking for and we get a better visual of the result (included some cleanup of stray code).

Suggested change

const { body } = await server.executeHTTPGraphQLRequest(request);

expect(JSON.stringify(body)).toMatchInlineSnapshot(

`"{\"kind\":\"complete\",\"string\":\"{\\n \\\"data\\\": {\\n \\\"hello\\\": \\\"stringifyResults works!\\\"\\n }\\n}\"}"`,

);

// const result = singleResult(body);

// expect(result.errors).toBeUndefined();

// expect(result.data?.hello).toBe('world');

const { body } = await server.executeHTTPGraphQLRequest(request);

assert(body.kind === 'complete');

expect(body.string).toMatchInlineSnapshot(`

"{

"data": {

"hello": "stringifyResults works!"

}

}"

`);

Gotcha, this is what i was hoping would happen. But, I didn't realize i needed this line:

assert(body.kind === 'complete');

Thanks for my walking me through it!

…ving commented out lines

trevor-scheer

Last round of feedback, looking good.

trevor-scheer · 2023-07-11T03:23:38Z

packages/server/src/runHttpQuery.ts

+        string: internals.stringifyResult
+          ? internals.stringifyResult(graphQLResponse.body.singleResult)
+          : prettyJSONStringify(
+              orderExecutionResultFields(graphQLResponse.body.singleResult),
+            ),


Sorry I didn't catch this earlier. I'd like to preserve the orderExecutionResultFields call in both cases since that's a spec concern. So maybe we do this instead, let me know what you thing:

make the internals.stringifyResult not optional (keep the configuration optional)

default the internals property to prettyJSONStringify if it's not configured

change the call here to just:

string: internals.stringifyResult( orderExecutionResultFields( graphQLResponse.body.singleResult, ), )

Makes sense to me -- making change now

Take a peak? :)

trevor-scheer

LGTM!

dfperry5 · 2023-07-11T20:58:44Z

@trevor-scheer -- Thanks so much! I'll keep an eye out for some other easier ones, unless you have some recommendations!

@dfperry5

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @apollo/server@4.8.0 ### Minor Changes - [#7634](#7634) [`f8a8ea08f`](f8a8ea0) Thanks [@dfperry5](https://github.com/dfperry5)! - Updating the ApolloServer constructor to take in a stringifyResult function that will allow a consumer to pass in a function that formats the result of an http query. Usage: ```ts const server = new ApolloServer({ typeDefs, resolvers, stringifyResult: (value: FormattedExecutionResult) => { return JSON.stringify(value, null, 2); }, }); ``` ## @apollo/server-integration-testsuite@4.8.0 ### Patch Changes - [#7649](#7649) [`d33acdfdd`](d33acdf) Thanks [@mastrzyz](https://github.com/mastrzyz)! - Add missing `supertest` dependency - [#7632](#7632) [`64f8177ab`](64f8177) Thanks [@renovate](https://github.com/apps/renovate)! - Update graphql-http dependency - Updated dependencies \[[`f8a8ea08f`](f8a8ea0)]: - @apollo/server@4.8.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

dfperry5 added 4 commits July 1, 2023 12:40

Issue-4359: Updating to expose a function to stringify the graphQL re…

3edcbbc

…sult in the http query response

Issue-4359: Testing stringify result parameter

e7e726c

Issue-4359: Cleaning up tests (removing .only)

bfb63d3

Merge branch 'apollographql:main' into Issue-4359-Pretty-Print-Response

271a07b

dfperry5 mentioned this pull request Jul 8, 2023

Pretty-print response #4359

Closed

dfperry5 added 5 commits July 8, 2023 10:54

Issue-4359: Removing commented out code in test

9770a5c

Merge branch 'Issue-4359-Pretty-Print-Response' of github.com:dfperry…

e043d27

…5/apollo-server into Issue-4359-Pretty-Print-Response

Issue-4359: Updating to have appropiate linting

eba009c

Issue-4359: Removing accidental typo

29bc192

Issue-4359: Updating to be in alignment with Prettier configs

59a8e4c

trevor-scheer requested changes Jul 10, 2023

View reviewed changes

dfperry5 added 2 commits July 10, 2023 17:52

Issue-4359: adding in test cases

2408db5

Issue-4359: Adding in changeset

66bf173

dfperry5 added 2 commits July 10, 2023 18:10

Issue-4359: Updating snapshots

48b1d5b

Issue-4359: Removed Deprecated snapshots

10b44be

trevor-scheer requested changes Jul 10, 2023

View reviewed changes

dfperry5 added 2 commits July 10, 2023 18:23

Issue-4359: Updating with prettier

9ee1620

Issue-4359: Removing integration tests and associated snapstops, upda…

4049e9c

…ting changeset file

trevor-scheer requested changes Jul 10, 2023

View reviewed changes

dfperry5 added 3 commits July 10, 2023 19:47

Issue-4359: Updating test case to be simpler and easy to read

3bc750c

Issue-4359: Updating to have better naming of the test case, and remo…

31b688c

…ving commented out lines

Issue-4359: Removed stray semi-colon in changeset file

bcbca2d

trevor-scheer requested changes Jul 11, 2023

View reviewed changes

dfperry5 added 2 commits July 11, 2023 09:41

Issue-4359: Defaulting stringifyResult to prettyJSONStringify

714c94d

Issue-4359: Call-site cleanup, and prettier

337b432

trevor-scheer approved these changes Jul 11, 2023

View reviewed changes

trevor-scheer merged commit f8a8ea0 into apollographql:main Jul 11, 2023
15 checks passed

github-actions bot mentioned this pull request Jul 11, 2023

Version Packages #7635

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 4359 pretty print response #7634

Issue 4359 pretty print response #7634

dfperry5 commented Jul 8, 2023

apollo-cla commented Jul 8, 2023

netlify bot commented Jul 8, 2023 •

edited

codesandbox-ci bot commented Jul 8, 2023 •

edited

dfperry5 commented Jul 8, 2023

trevor-scheer left a comment

trevor-scheer Jul 10, 2023

trevor-scheer commented Jul 10, 2023

dfperry5 commented Jul 10, 2023

trevor-scheer Jul 10, 2023

trevor-scheer Jul 10, 2023

dfperry5 Jul 10, 2023

trevor-scheer commented Jul 10, 2023

dfperry5 commented Jul 10, 2023

trevor-scheer left a comment

trevor-scheer Jul 10, 2023

dfperry5 Jul 10, 2023

trevor-scheer Jul 10, 2023

dfperry5 Jul 10, 2023

trevor-scheer Jul 10, 2023

dfperry5 Jul 10, 2023 •

edited

trevor-scheer left a comment

trevor-scheer Jul 11, 2023

dfperry5 Jul 11, 2023

dfperry5 Jul 11, 2023

trevor-scheer left a comment

dfperry5 commented Jul 11, 2023

	return JSON.stringify(value, null, 2);;
	return JSON.stringify(value, null, 2);

Issue 4359 pretty print response #7634

Issue 4359 pretty print response #7634

Conversation

dfperry5 commented Jul 8, 2023

apollo-cla commented Jul 8, 2023

netlify bot commented Jul 8, 2023 • edited

👷 Deploy request for apollo-server-docs pending review.

codesandbox-ci bot commented Jul 8, 2023 • edited

dfperry5 commented Jul 8, 2023

trevor-scheer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevor-scheer commented Jul 10, 2023

dfperry5 commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevor-scheer commented Jul 10, 2023

dfperry5 commented Jul 10, 2023

trevor-scheer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfperry5 Jul 10, 2023 • edited

Choose a reason for hiding this comment

trevor-scheer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevor-scheer left a comment

Choose a reason for hiding this comment

dfperry5 commented Jul 11, 2023

netlify bot commented Jul 8, 2023 •

edited

codesandbox-ci bot commented Jul 8, 2023 •

edited

dfperry5 Jul 10, 2023 •

edited