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

Add DocumentNode to gql declaration #196

Merged
merged 1 commit into from
Jul 19, 2018
Merged

Add DocumentNode to gql declaration #196

merged 1 commit into from
Jul 19, 2018

Conversation

kamilkisiela
Copy link
Contributor

Pull Request Labels

  • feature
  • blocking
  • docs

@ghost ghost added the feature New addition or enhancement to existing solutions label Jul 11, 2018
@kamilkisiela kamilkisiela requested a review from jnwng July 18, 2018 12:21
@yordis
Copy link

yordis commented Jul 19, 2018

@kamilkisiela what was what I did on #193 😄

@jnwng
Copy link
Contributor

jnwng commented Jul 19, 2018 via email

@yordis
Copy link

yordis commented Jul 19, 2018

@jnwng no, this would actually help people to remove the any type and have better typing support. Anyway before everyone would use any, so no breaking changes.

@kamilkisiela
Copy link
Contributor Author

It's not, DocumentNode is a part of graphql since a long time and if someone uses TypeScript and graphql he has a proper type definitions for that too.

@jnwng jnwng merged commit ae792b6 into master Jul 19, 2018
@kamilkisiela kamilkisiela deleted the ts/document-node branch July 19, 2018 07:57
@jnwng jnwng mentioned this pull request Jul 19, 2018
3 tasks
@jnwng
Copy link
Contributor

jnwng commented Jul 20, 2018

btw @kamilkisiela @yordis i thought i had seen this before — this was previously introduced by #141 and subsequently had to be reverted because this is a breaking change (see #150)

i'm not going to release this just yet — while im not afraid of cutting a major version here, i could use some help understanding why this a breaking change at all given that both of you mentioned that it isn't. any helpful information for TypeScript users here will help us head off unnecessary pain.

jnwng added a commit that referenced this pull request Oct 2, 2018
This reverts commit ae792b6. Based on #196 and #150, introducing the `DocumentNode` return value causes this library to break for existing TypeScript installations. I'm reverting this temporarily so we can release support for GraphQL v14 in a minor patch, and if we can't fix the TypeScript configuration, we'll release a major version with this commit back in.
jnwng added a commit that referenced this pull request Oct 2, 2018
This reverts commit ae792b6. Based on #196 and #150, introducing the `DocumentNode` return value causes this library to break for existing TypeScript installations. I'm reverting this temporarily so we can release support for GraphQL v14 in a minor patch, and if we can't fix the TypeScript configuration, we'll release a major version with this commit back in.
hwillson added a commit that referenced this pull request 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 added a commit that referenced this pull request 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 added a commit that referenced this pull request Feb 4, 2020
* Update/fix the existing TS declaration file

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.

* Changelog update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants