Issues with the dedent string tag used for testing

[Cito]

GraphQL.js has a ES6 string tag helper function under jsutils/dedent that removes indentation from strings so that the unit tests can look nicer.

One undocumented side effect of the dedent function however confused me when reading some of the tests: The tag also adds escape sequences to the dedented string. Take for example this test:

  it('Prints String Field With String Arg With Default', () => {
    const output = printSingleFieldSchema({
      type: GraphQLString,
      args: { argOne: { type: GraphQLString, defaultValue: 'tes\t de\fault' } },
    });
    expect(output).to.equal(dedent`
      schema {
        query: Root
      }

      type Root {
        singleField(argOne: String = "tes\t de\fault"): String
      }
    `);
  });

The output actually contains escape sequences, like "tes\\t de\\fault", but the test looks likes as if it expects the output to be a non-escaped string.

The reason for this is that the dedent tag accesses string.raw instead of string.

I'm not sure whether this is intended or not. If it is, then it should at least be documented in jsutils/dedent, because having such a side effect is not what you expect from a function called "dedent". It should also be documented that the function only looks at spaces, it does not dedent tab-based indentation.

If you me know whether you prefer fixing the documentation or fixing the behavior, I can create a PR.

[IvanGoncharov]

@Cito I think you right, only a handful of tests contains escape symbols so they can be changed to:

dedent(String.raw `tes\t de\fault`)

This form is more explicit and less confusing.

It should also be documented that the function only looks at spaces, it does not dedent tab-based indentation.

It's very easy to add tab support so I think it should be added even though this function only used in tests and they are indented only with spaces.

[Cito]

Right, I would also prefer the dedent+String.raw form which is more explicit. Or you could just use double backslashes in the string which also makes it clear that the output is expected in escaped form.

And yes, adding tab support is probably as easy as changing the regex in the first line of fixIndent().

Do you want me to create a PR that changes this behavior of dedent, i.e. returns the normal "cooked" string instead of the raw string and supports tabs as well? I would then also add a unit test for the dedentfunction, and adapt the tests that are using dedent with escaped chars.

[IvanGoncharov]

Do you want me to create a PR that changes this behavior of dedent, i.e. returns the normal "cooked" string instead of the raw string and supports tabs as well?

I'm for it 👍
But PR still need to be approved by @leebyron

[Cito]

Ok. See PR #1203 for the discussed changes and unit tests.