Skip to content
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.

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

[gql-tag-operations-preset] Generated index.ts should contain \n regardless of whether source contains LF or CRLF #7362

Closed
vyacheslav-pushkin opened this issue Jan 12, 2022 · 6 comments

Comments

@vyacheslav-pushkin
Copy link
Contributor

First of all, I would like to thank the creators of gql-tag-operations-preset, it's a really clever solution to have type safety while avoiding explicit imports.

Describe the bug

TL;DR

Generated index.ts will cause TypeScript compilation to fail when source files contain CRLF (e.g. when using Windows).

Long description

When source file contains LF, generated index.ts will contain \n and TypeScript will compile successfully.

When source file contains CRLF, generated index.ts will contain \r\n and TypeScript will not compile.

When source file contains CRLF, and generated index.ts is manually changed to contain \n, TypeScript will compile successfully.

It seems that generated index.ts should contain \n regardless of whether the source contains LF or CRLF.

To Reproduce

NOTE: I have created a minimal reproduction example. It contains schema, codegen.yml and operations.

Steps to reproduce the problem (using Linux):

1. Modify src/app/app.tsx by replacing LF with CRLF:

$ unix2dos src/app/app.tsx 
unix2dos: converting file src/app/app.tsx to DOS format...

New lines are now CRLF:

$ cat -A src/app/app.tsx
import {gql} from "@my/gql";^M$
import {useQuery} from "@apollo/client";^M$
import React from "react";^M$
^M$
const PET_LIST = gql(`^M$
  query Get_Pet_List {^M$
    petList {^M$
      id^M$
      identificationNumber^M$
    }^M$
  }^M$
`);^M$
^M$
export const MyComponent = () => {^M$
  const {data} = useQuery(PET_LIST);^M$
^M$
  return <>{data?.petList?.[0]?.id}</>;^M$
};^M$

2. Run graphql-codegen by executing npm run generate:

$ npm run generate

> [email protected] generate
> graphql-codegen

(node:9963) ExperimentalWarning: stream/web is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
  ✔ Parse configuration
  ✔ Generate outputs

The generated index.ts file will now contain \r\n:

$ cat src/gql/index.ts
/* eslint-disable */
import * as graphql from './graphql';
import { TypedDocumentNode as DocumentNode } from '@graphql-typed-document-node/core';

const documents = {
    "\r\n  query Get_Pet_List {\r\n    petList {\r\n      id\r\n      identificationNumber\r\n    }\r\n  }\r\n": graphql.Get_Pet_ListDocument,
};

. . .

3. Running tsc will now result in a compilation error:

$ tsc
src/app/app.tsx:15:27 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'DocumentNode | TypedDocumentNode<any, Record<string, any>>'.
  Type 'unknown' is not assignable to type 'TypedDocumentNode<any, Record<string, any>>'.

15   const {data} = useQuery(PET_LIST);
                             ~~~~~~~~


Found 1 error.

4. Now, if you go to generated index.ts and manually change all \r\n to \n and run tsc again - it will compile.

Environment

  • "@graphql-codegen/cli": "^2.3.1",
  • "@graphql-codegen/gql-tag-operations-preset": "^1.1.7",
  • "@graphql-codegen/typescript": "^2.2.2",
  • NodeJS: 16.13.1

Additional info

This ticket suggests configuring .gitattributes to force using LF instead of CRLF on Windows. However, in my opinion it is not reasonable to ask users to change their CRLF strategy in order to work around this, it seems too invasive. Also, unless I'm missing something, it won't seem to help with source files created on Windows machine.

@n1ru4l
Copy link
Collaborator

n1ru4l commented Jan 13, 2022

We had a similar issue reported in #6811, and the following seems to be the most reasonable approach of solving it: #6811 (comment)

Do you agree with that, would you like to help out documenting that or exploring alternative solutions?

@vyacheslav-pushkin
Copy link
Contributor Author

Hi @n1ru4l

I don't think #6811 (comment) is the best solution because it won't help when a new source file is created on Windows.

I can offer my help working on this issue.

Empirically it seems that outputting \n works for both CRLF and LF source files. I will look into it more deeply. If you have any thoughts on that - it would be most helpful.

@vyacheslav-pushkin
Copy link
Contributor Author

It's seems that TypeScript does not differentiate between CRLF and LF - both become \n.

// This template literal contains CRLF as line breaks
type CRLF = `one
two
`;

// This template literal contains LF as line breaks
type LF = `one
two
`;

const test1: CRLF = 'one\r\ntwo\n'; // TS2322: Type '"one\r\ntwo\n"' is not assignable to type '"one\ntwo\n"'.
const test2: LF = 'one\r\ntwo\n'; // TS2322: Type '"one\r\ntwo\n"' is not assignable to type '"one\ntwo\n"'.
const test3: CRLF = 'one\ntwo\n'; // compilation passes
const test4: LF = 'one\ntwo\n'; // compilation passes

I'll work on a PR.

vyacheslav-pushkin added a commit to vyacheslav-pushkin/graphql-code-generator that referenced this issue Jan 14, 2022
vyacheslav-pushkin added a commit to vyacheslav-pushkin/graphql-code-generator that referenced this issue Jan 14, 2022
vyacheslav-pushkin added a commit to vyacheslav-pushkin/graphql-code-generator that referenced this issue Jan 17, 2022
vyacheslav-pushkin added a commit to vyacheslav-pushkin/graphql-code-generator that referenced this issue Jan 17, 2022
n1ru4l pushed a commit that referenced this issue Jan 17, 2022
* test: add test reproducing CRLF issue #7362

* fix: gql-tag-operations generates invalid types on Windows #7362

* chore: add changeset
@n1ru4l
Copy link
Collaborator

n1ru4l commented Jan 17, 2022

Fixed via #7369

@jmarbutt
Copy link

I am having a similar issue, is this happening again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants