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

Update/fix the existing TS declaration file #285

Merged
merged 2 commits into from
Feb 4, 2020
Merged

Conversation

hwillson
Copy link
Member

@hwillson hwillson commented Feb 4, 2020

This PR modernizes/fixes the existing TypeScript declaration file as follows:

  • The module declaration now better represents how exports are currently setup, by using module augmentation to give access to helper functions that are added to the gql function itself.
  • Backwards compatibility has been maintained by re-exporting helper functions.
  • Stronger typing changes; the first param of the gql template literal is now a read only string based array, and the gql return type is a DocumentNode. We're leveraging TS' import types functionality to reference the DocumentNode type, which will help avoid issues like the previously reverted attempts to do this in Reverting #141 until we can figure out why its throwing errors. #151 and Add DocumentNode to gql declaration #196.
  • While the new gql type changes could in theory be considered breaking, they really shouldn't be in practice. Tagged template literals enforce the use of an immutable array of strings, which means that even though literals was any before, the param should only ever have been an array of strings. Along the same lines, the gql function has only ever returned a DocumentNode, so moving away from the any return type should be pretty safe.

Fixes #282
Fixes #150

@hwillson hwillson self-assigned this Feb 4, 2020
This commit modernizes/fixes the existing TypeScript declaration
file as follows:

- The module declaration now better represents how exports are
  currently setup, by using module augmentation to give access to
  helper functions that are added to the `gql` function itself.
- Backwards compatibility has been maintained by re-exporting helper
  functions.
- Stronger typing changes; the first param of the `gql` template
  literal is now a read only string based array, and the `gql` return
  type is a `DocumentNode`. We're leveraging TS'
  [import types](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-9.html#import-types)
  functionality to reference the `DocumentNode` type, which will
  help avoid issues like the previously reverted attempts to do this
  in #151 and
  #196.
- While the new `gql` type changes could in theory be considered
  breaking, they really shouldn't be in practice. Tagged template
  literals enforce the use of an immutable array of strings, which
  means that even though `literals` was `any` before, the param
  should only ever been an array of strings. Along the same lines,
  the `gql` function has only ever returned a `DocumentNode`, so
  moving away from the `any` return type should be pretty safe.
@hwillson hwillson merged commit 385d630 into master Feb 4, 2020
@hwillson hwillson deleted the ts-declaration-updates branch February 4, 2020 10:52
hwillson added a commit to apollographql/apollo-client that referenced this pull request Feb 4, 2020
The `graphql-tag` declaration file has been updated in
apollographql/graphql-tag#285 so we no
longer need our own overrides.
@SoyYoRafa
Copy link

SoyYoRafa commented Feb 4, 2020

This breaks a lot of code. It shouldn't have been published as a patch version.

export const myDoc = gql`...`

Now generates a TypeScript compilation error The inferred type of "myDoc" cannot be named without a referenced to "...graphql-tag/node_modules/graphql". This is likely not portable. A type annotation is necessary. I don't think that the code above is wrong so this PR must have created an issue.

@hwillson
Copy link
Member Author

hwillson commented Feb 5, 2020

@SoyYoRafa what version of TypeScript are you using?

@SoyYoRafa
Copy link

3.7.5 with this tsconfig.json:

{
  "compilerOptions": {
    "forceConsistentCasingInFileNames": true,
    "target": "es2018",
    "module": "commonjs",
    "strict": true,
    "strictNullChecks": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noUnusedLocals": true,
    "lib": ["esnext.asynciterable", "es2018", "dom"],
    "sourceMap": true,
    "declaration": true,
    "declarationMap": true,
    "allowSyntheticDefaultImports": true,
    "experimentalDecorators": true
  }
}

@hwillson
Copy link
Member Author

hwillson commented Feb 5, 2020

Thanks @SoyYoRafa - using the import type of import("graphql").DocumentNode should help avoid this error message. We haven't had any other reports of this being an issue since this PR was merged and released, so if there's any chance you can put together a small runnable repro that shows this, that would be helpful.

hwillson added a commit to apollographql/apollo-client that referenced this pull request Feb 9, 2020
The `graphql-tag` declaration file has been updated in
apollographql/graphql-tag#285 so we no
longer need our own overrides.
@SoyYoRafa
Copy link

Sure, how do you want me to share the reproduction? Pretty this is due to graphql-tag referencing the graphql package in the declaration file and not marking graphql as a dependency.

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

Successfully merging this pull request may close these issues.

Improve typescript definitions 2.7.0 breaks existing typescript code
2 participants